Skip to content

Add support for pinning snapshots in gradle resolver #1412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

acecilia
Copy link

@acecilia acecilia commented Jul 11, 2025

Hi 👋

This PR adds support for snapshot dependencies in the gradle resolver. It uses the repository timestamp of the snapshot as a version. I added this information in a new field with a generic name VersionRevision.

Related: #974

String pomName = String.format("%s-%s.pom", coords.getArtifactId(), coords.getVersion());
String pom = Paths.get(originalTarget).getParent().resolve(pomName).toString();

String pom = coords.setExtension("pom").toRepoPath();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the special handling of POM files above 🤔

@acecilia acecilia force-pushed the 3-snapshot_support branch from d2b0d98 to 7a13a1d Compare July 11, 2025 02:21
@acecilia acecilia marked this pull request as ready for review July 11, 2025 02:48
@acecilia acecilia requested review from jin, shs96c and cheister as code owners July 11, 2025 02:48
"jar": "79abe7953803b20d1deb5f36283d4d640c7aacbdb1ddd7601aae369b1633e903"
},
"version": "999.0.0-HEAD-jre-SNAPSHOT",
"version_revision": "999.0.0-HEAD-jre-20250623.150948-114"
Copy link
Author

@acecilia acecilia Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is how the snapshot version pinning looks like, including the timestamp

@shs96c
Copy link
Collaborator

shs96c commented Jul 11, 2025

Thank you for the PR. I think there's a couple of things that I think about as I read this:

  1. Snapshots aren't always stored with a version. Sometimes, the -SNAPSHOT.jar is just silently replaced
  2. We should have a consistent story for handling snapshots throughout the ruleset

You are correct that some maven repos do write snapshots versions like this, and that's what makes things tricky. I suspect that the correct fix will be to:

  1. Identify if a dep is a snapshot in some way. I think a boolean field is enough for that, and we can add it only for snapshots. For versioned snapshots, we might not need to do this, as I think they get stored in maven repos as the url expansions expect.
  2. When we create the lockfile, omit the sha256 of any snapshot jar that doesn't have a fixed version (likely still contains SNAPSHOT.jar in the file name). That means that if the version is updated in place, builds won't break unexpectedly.

What do you think?

@acecilia
Copy link
Author

acecilia commented Jul 11, 2025

Thanks for the feedback. Let me ask for some more context to understand a bit better your thoughts 😄

Snapshots aren't always stored with a version. Sometimes, the -SNAPSHOT.jar is just silently replaced

By "version", do you mean "timestamp"? If yes, is there maybe a public snapshot you could share that does not have timestamp, that I could use during development and to add tests?

We should have a consistent story for handling snapshots throughout the ruleset

What do you mean here, that snapshots should be handled by all resolvers? If yes, I agree. I was planning to look into the other resolvers after merging this PR. Is that okey, or would you prefer to do it differently?

When we create the lockfile, omit the sha256 of any snapshot jar that doesn't have a fixed version (likely still contains SNAPSHOT.jar in the file name). That means that if the version is updated in place, builds won't break unexpectedly.

Let me expand a bit on my comment here. How would this work in bazel?

  • Bazel uses http_file rule in here to download the artifacts. That rule has a sha256 property that is optional, but the docs mention that It is a security risk to omit the SHA-256 as remote files can change. At best omitting this field will make your build non-hermetic - we would be accepting these risks when adding support for such kind of snapshots
  • For http_file rules without sha256, bazel would keep the downloaded artifacts in the cache, and will never try to update them. How do you envision that someone would trigger a snapshot update in this case? My understanding is that gradle caches snapshots but still fetches them once a day - which is not the case for bazel
  • My current understanding is that time-stamping snapshots is the default behaviour for maven repositories. I acknowledge that when searching in google "maven snapshots without timestamp" there are some ways to do it mentioned (example here and here), but I also observed that those mentions are usually +10 years old. Furthermore, when trying to find more recent resources about how to do this, I found this from 3 years ago that mentions that Support for uniqueVersion was deprecated in Maven 2.x, and removed entirely in Maven 3, which is confirmed by maven documentation in here.

Integrating snapshots without timestamps goes against bazel fundamentals + it is deprecated from maven 3. This is what makes me question wether this ruleset wants/needs to support it. Wdyt?

@acecilia acecilia force-pushed the 3-snapshot_support branch from 1f1457a to e16bd7b Compare July 11, 2025 13:17
@acecilia
Copy link
Author

@shs96c let me know 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants