✨ feat: Add resolved field by LogFlames · Pull Request #1148 · chains-project/maven-lockfile

LogFlames · 2025-03-26T15:46:57Z

LogFlames · 2025-03-28T17:31:32Z

Will solve #1149 first.

LogFlames · 2025-04-06T00:16:58Z

TODOs:

Add RemoteChecksumCalculator
Add checks to IT to check for correctly ResolvedUrl

LogFlames · 2025-04-08T08:25:42Z

This PR is now ready for review, became quite a lot of lines changed as all integration tests had to be updated with the new resolved field.

algomaster99

Overwhelming PR but great job. I have some comments on the implementation and some changes.

algomaster99 · 2025-04-08T08:44:58Z

+public class DeprecationUtils {
+    public static String ChecksumModeDeprecation(String checksumMode, Log logger) {
+        if (checksumMode.equals("maven_local")) {
+            if (logger != null) {
+                logger.warn("Option 'checksumMode=maven_local' is deprecated. Use 'checksumMode=local' instead.");
+            }
+            return "local";
+        }
+        if (checksumMode.equals("maven_central")) {
+            if (logger != null) {
+                logger.warn("Option 'checksumMode=maven_central' is deprecated. Use 'checksumMode=remote' instead.");
+            }
+            return "remote";
+        }
+
+        return checksumMode;
+    }


Why do we need this wrapper? Does't the logging library support @Deprecated annotation?

Would @deprecated be used to generate the warning message for the log? Or to mark the option?

We can deprecate a parameter, which in this case would be checksumMode. But it is not the parameter which is deprecated but the options for it.

Looking at it I don't think it is needed to separate this into its own thing. I wanted to make sure the upgrade from older versions of lockfiles were smooth but since the config is generated from Gson it doesn't seem to go through the Config constructor and could therefore not be automatically updated, will revert.

(P.s. sorry @ deprecated, not ment to mention you)

generate the warning message for the log? Or to mark the option?

Ideally, there should be a warning message at runtime also but I am not sure if @Deprecated provides that - it is only a compiler warning. I assumed the logging libraries would also support it and can log the usage of deprecated components.

algomaster99 · 2025-04-08T09:37:35Z

+            Path remoteRepositoriesPath = artifactFolderPath.resolve("_remote.repositories");
+            List<String> locallySavedRemoteRepositories = Files.readAllLines(remoteRepositoriesPath);


I feel that since we depend upon maven resolver, we should have the URL information readily available. We have access to org.apache.maven.artifact.Artifact which has an API getDownloadUrl and I expected it to returned the "resolved URL" but somehow it returns null.

Steps I tried:

mvn clean install -DskipTests mvnDebug io.github.chains-project:maven-lockfile:5.4.3-SNAPSHOT:generate # on any maven project with plugins and dependencies

project.getPluginArtifactMap().get("org.cyclonedx:cyclonedx-maven-plugin").getDownloadUrl() // returns null

I also tried deleting org.cyclonedx:cyclonedx-maven-plugin artifact from my .m2 folder but downloadUrl is still null. Probably a bug in maven?

I am up for implementation using _remote.repositories file, but like you mentioned that day this file is internal to maven. I would prefer an implementation where artifact comes with the URL it was downloaded using.

Fully agree an implementation using public APIs is preferable!

I also tried a bunch to get the value out of the getDownloadUrl as well, when I was speaking about not finding it in the open API.

My best guess for why it is null is that maven resolves and downloads the dependency prior to plugins running and that we would need to create a maven core extension to access this information. However, we would not want to be forced to redownload all dependencies to capture their resolved url so we would still need to read this file or access other internals of maven resolver.

Maybe we could send a mail to a mailing list and ask if someone knows how to access the download url with a public API from a plugin? Apache dev?

the dependency prior to plugins running and that we would need to create a maven core extension to access this information.

I think the URL for dependencies was also null.

Maybe we could send a mail to a mailing list and ask if someone knows how to access the download url with a public API from a plugin? Apache dev?

Do ask them 👍 Maven Developer's List seems okay.

algomaster99 · 2025-04-08T09:44:58Z

It seems to have a lot of already merged commits. Could you rebase this branch with main?

…in to v3.4.1 (#1103) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: LogFlames <36220731+LogFlames@users.noreply.github.com>

…ositories

RemoteChecksumCalculator is still left to update.

LogFlames · 2025-04-08T11:03:04Z

Ill try a rebase, hopefully it'll go well ^^ Long history 😅

algomaster99 · 2025-04-08T21:34:19Z

Try converting the above class into record. We can merge this then. The implementation can be improved later according to what email response we get.

algomaster99 · 2025-04-16T08:59:55Z

We merge after updating README.

…ded-artifact-to-the-lockfile

Ensure consistency between RemoteChecksumCalculator and FileChecksumCalculator. See comment in #1149.

LogFlames · 2025-04-16T09:33:44Z

@algomaster99 @massimeddu-sj note changes in RemoteChecksumCalculator to ensure consistency between RemoteChecksumCalcuclator and FileSystemChecksumCalculator when a checksum cannot be found.
See comment in #1149.

…ded-artifact-to-the-lockfile

LogFlames linked an issue Mar 26, 2025 that may be closed by this pull request

add resolved field with url to a downloaded artifact to the lockfile #995

Closed

LogFlames marked this pull request as draft March 26, 2025 15:47

LogFlames marked this pull request as ready for review April 8, 2025 07:04

LogFlames requested a review from algomaster99 April 8, 2025 08:25

algomaster99 requested changes Apr 8, 2025

View reviewed changes

renovate Bot and others added 21 commits April 8, 2025 12:38

⬆️(deps): Update dependency org.apache.maven.plugins:maven-clean-plug…

2dc1952

…in to v3.4.1 (#1103) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: LogFlames <36220731+LogFlames@users.noreply.github.com>

experiments with repostiory listeners

8b4652f

Start adding resolved to lockfile

71dae3b

Read _remote.repositories

91ddda9

📝 Update Documentation with current version

6cf511d

Start to build buildingRequest from project instead of session

100d609

📝 Update Documentation with current version

fbf1dd1

Remove logging repository listener

9d1e522

Split filechecksumcalculator into plugin and artifact, add remote rep…

4f76cdf

…ositories

Clearer passing of buildingRequest

f6acadd

Add remote checksum mode test

78116c6

📝 Update Documentation with current version

b92c490

Move retry to maven-failsafe-plugin, fix spelling error

2057dc0

Remove unused imports

505b4f1

Resolve URL for FileSystemChecksumCalculator

20f6580

RemoteChecksumCalculator is still left to update.

📝 Update Documentation with current version

5062572

Update lockfiles in IT

2f7cf6e

Remove resolve field from lockfile validation and hash ordering

5a41deb

Update testcase to failing input

21186f6

Resolve url in remote checksum calculator

16f8002

📝 Update Documentation with current version

7b327f2

LogFlames added 3 commits April 8, 2025 12:43

Retrigger CI

6de47e7

Add IT that checks resolved url

0608bd9

Move deprecation to separate file

c2542e4

LogFlames mentioned this pull request Apr 8, 2025

Use Record classes in data/ #1168

Closed

LogFlames force-pushed the 995-add-resolved-field-with-url-to-a-downloaded-artifact-to-the-lockfile branch from b9ca86c to c2542e4 Compare April 8, 2025 11:05

LogFlames and others added 5 commits April 8, 2025 11:06

chore: update lockfile

77a761a

Remove lockfile diff

88ea529

chore: update lockfile

8d9ebda

Revert deprecationutils, use function in method

e7c2ddc

Update comment

eb70712

LogFlames and others added 5 commits April 9, 2025 08:58

Use record class

3f72b6e

📝 Update Documentation with current version

1aa35ef

retrigger ci

417e38e

Upgrade project source version

2a90377

Revert from record class

177ccb0

Merge branch 'main' into 995-add-resolved-field-with-url-to-a-downloa…

6200703

…ded-artifact-to-the-lockfile

LogFlames mentioned this pull request Apr 16, 2025

✨ feat: Resolve dependencies from other repositories than maven central #1151

Merged

LogFlames and others added 3 commits April 16, 2025 11:30

Do not throw RuntimeException if remote checksum calc fails.

21dad38

Ensure consistency between RemoteChecksumCalculator and FileChecksumCalculator. See comment in #1149.

Update README with resolve field in lockfile example

1009b72

📝 Update Documentation with current version

2791e9e

Retrigger CI

1d7ceee

algomaster99 requested changes Apr 16, 2025

View reviewed changes

Comment thread README.md

LogFlames and others added 2 commits April 16, 2025 12:32

Document unresolved or missing checksums in README

b17f89e

Merge branch 'main' into 995-add-resolved-field-with-url-to-a-downloa…

9386bbf

…ded-artifact-to-the-lockfile

LogFlames merged commit c73c947 into main Apr 16, 2025

LogFlames deleted the 995-add-resolved-field-with-url-to-a-downloaded-artifact-to-the-lockfile branch April 25, 2025 12:32

		Path remoteRepositoriesPath = artifactFolderPath.resolve("_remote.repositories");
		List<String> locallySavedRemoteRepositories = Files.readAllLines(remoteRepositoriesPath);

Uh oh!

Conversation

LogFlames commented Mar 26, 2025

Uh oh!

LogFlames commented Mar 28, 2025

Uh oh!

LogFlames commented Apr 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LogFlames commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

algomaster99 left a comment

Choose a reason for hiding this comment

Uh oh!

algomaster99 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

LogFlames Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

LogFlames Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

algomaster99 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

algomaster99 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

LogFlames Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

algomaster99 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

algomaster99 commented Apr 8, 2025

Uh oh!

LogFlames commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

algomaster99 commented Apr 8, 2025

Uh oh!

algomaster99 commented Apr 16, 2025

Uh oh!

LogFlames commented Apr 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LogFlames commented Apr 6, 2025 •

edited

Loading

LogFlames commented Apr 8, 2025 •

edited

Loading

LogFlames Apr 8, 2025 •

edited

Loading

LogFlames commented Apr 8, 2025 •

edited

Loading