smithy-lang/smithy

Snapshot dependencies

kubukoz opened this issue · 4 comments

Just updated to 1.34.0, and I bumped into this:

https://github.com/awslabs/smithy/blob/1ce05b2640eb006c737d8cb278a33ca2c31ad821/smithy-cli/src/main/java/software/amazon/smithy/cli/dependencies/MavenDependencyResolver.java#L155-L157

Snapshot dependencies not being supported seems like a very strange limitation - we used to rely on snapshots a lot for developing new features in a Smithy library, while being able to test out things like packaging, LSP support, etc. without committing to a stable release.

Why is this requirement in there? Could it be relaxed? After the recent change to the LSP, which now uses this custom resolver rather than Coursier (which allows snapshots), it's going to be a problem for orgs that share Smithy libraries (e.g. for custom protocols) between repositories.

Snapshot dependencies make it impossible to create a reproducible build, so we disabled support for it until we can provide something like a lock file for Smithy projects.

I understand. To be precise, our snapshots include the commit hash and (if there's no commit) the timestamp of the artifact, that way you don't get a different artifact on repeated runs.

While I see the concerns you might have against allowing snapshots, should it really be a concern of the dependency resolver? I consider it more of a build/packaging/validation/publishing step. Why not make it something that happens at a later phase instead?

In my usecase, I could easily prohibit my users from using snapshot dependencies for real PRs, but for in-progress ones it very often makes sense to allow these (to a certain point in the build). I'm happy to share more context if that helps.

I agree with @kubukoz : this limitation, as it is implemented in the dependency resolver, breaks our usage of the LSP as coursier was replaced by aether in this PR.

I can understand the desire to prevent non-reproducing builds, but implementing this prevention in the resolver breaks separation of concern, and impedes on experimentation workflows.

We can remove this validation to support snapshots.