Skip to content
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

338c666ba87aecf8d34cfee5b02ee3209b78e091 broke the jgit build #232

Closed
msohn opened this issue Apr 27, 2023 · 5 comments
Closed

338c666ba87aecf8d34cfee5b02ee3209b78e091 broke the jgit build #232

msohn opened this issue Apr 27, 2023 · 5 comments

Comments

@msohn
Copy link
Contributor

msohn commented Apr 27, 2023

JGit builds for its stable-6.4, stable-6.5 and master branch started failing with classpath issues since Tuesday Apr 25.
e.g. https://ci.eclipse.org/jgit/job/stable/job/jgit.gerrit-pipeline.java11/3577/console

I bisected the JGit history between stable-6.3 and master to identify which change caused the build failures
and found that jgit commit b398bb91ab28b799ea39dc2d1b5bd0b849392443
(https://git.eclipse.org/r/c/jgit/jgit/+/196333) is causing the problem.

This change configured the dash license-tool-plugin to fail the build if any dependency isn't yet license-approved. This commit was introduced on branch stable-6.4 and submitted on Oct 15, 2022. This worked for 6 months and started failing this Tuesday. I think the problem is that we depend on the 0.0.1-SNAPSHOT version of the plugin since it didn't yet publish a release. This means we depend on the latest build published for this plugin which can change any time.

I removed the pluginRepository link in jgit's pom.xml to the https://repo.eclipse.org/content/repositories/dash-licenses-snapshots/ Maven repo to use a local build of the license-tool-plugin. Bisecting its history yields that
338c666
broke the jgit builds on all branches which contain https://git.eclipse.org/r/c/jgit/jgit/+/196333.

Please fix this and start providing releases of the license-tool-plugin to help avoiding such hard to discover issues.

Also see the discussion on the jgit-dev list https://www.eclipse.org/lists/jgit-dev/msg04181.html

@dhendriks
Copy link

Commit 338c666 fixes #199, and I'm very happy with it. It works perfectly for Eclipse ESCET, see https://gitlab.eclipse.org/eclipse/escet/escet/-/merge_requests/454.

I'm not sure what is causing the classpath issues you have. I don't really see anything about the Dash license check tool in the output of the failed build at https://ci.eclipse.org/jgit/job/stable/job/jgit.gerrit-pipeline.java11/3577/console that relates to the errors you describe. Are you really sure the Dash license check tool is the problem, and that it not just masks some other underlying problem? Maybe the Dash license check tool added dependencies to your build before, and now they are gone, and you have classpath issues? The Dash license check tool should not lead to dependencies being found, it should only resolve them for its own use. At least, that is the change that was done, as far as I understand commit 338c666. Then maybe your build is missing some dependencies somewhere, that are the underlying cause of the classpath issues, and should be solved by adding the correct dependencies in the correct place, rather than blaming the Dash license check tool?

It seems the Dash license check plugin is executed for every plugin, not just the root project. We only check it for the root POM in Eclipse ESCET. Maybe that could also be causing part of the issue?

[...] start providing releases of the license-tool-plugin to help avoiding such hard to discover issues.

I think having releases that don't magically change is indeed a good idea.

@waynebeaton
Copy link
Member

I've reverted the commit, rebuilt/published the plugin, and will reopen #199. Many warnings is better than long-established builds getting broken (I think that we saw something similar with another build yesterday; I'll try to see if they're related). FWIW, issue #199 demonstrates that the configuration of a plugin can impact other plugins (that is, we can't necessary concluded that there is a problem with the JGit build)

Up to this point, the Eclipse Dash License Tool actually has no releases. But... you're right. It's time to add more rigour and make some actual releases.

I've initiated a progress review with the EMO.

@msohn
Copy link
Contributor Author

msohn commented May 1, 2023

thanks, the revert fixed our builds

@msohn msohn closed this as completed May 1, 2023
@msohn
Copy link
Contributor Author

msohn commented May 1, 2023

Commit 338c666 fixes #199, and I'm very happy with it. It works perfectly for Eclipse ESCET, see https://gitlab.eclipse.org/eclipse/escet/escet/-/merge_requests/454.

I'm not sure what is causing the classpath issues you have. I don't really see anything about the Dash license check tool in the output of the failed build at https://ci.eclipse.org/jgit/job/stable/job/jgit.gerrit-pipeline.java11/3577/console that relates to the errors you describe. Are you really sure the Dash license check tool is the problem, and that it not just masks some other underlying problem? Maybe the Dash license check tool added dependencies to your build before, and now they are gone, and you have classpath issues? The Dash license check tool should not lead to dependencies being found, it should only resolve them for its own use. At least, that is the change that was done, as far as I understand commit 338c666. Then maybe your build is missing some dependencies somewhere, that are the underlying cause of the classpath issues, and should be solved by adding the correct dependencies in the correct place, rather than blaming the Dash license check tool?

It seems the Dash license check plugin is executed for every plugin, not just the root project. We only check it for the root POM in Eclipse ESCET. Maybe that could also be causing part of the issue?

How do you do that ?
AFAICS you didn't yet integrate running the license-check in your maven build.
See https://gitlab.eclipse.org/pvberkel/escet/-/blob/develop/releng/org.eclipse.escet.releng.configuration/pom.xml#L355
Meanwhile the plugin can fail the build if the license check fails.

@dhendriks
Copy link

How do you do that ? AFAICS you didn't yet integrate running the license-check in your maven build. [...]

For about two years, we've been running that as a separate daily build, using a separate Jenkinsfile.

I have a merge request to integrate it into the main build. It has been open for about 4 months, waiting on the fix for the warnings. I was waiting for fellow committers to review it, when the fix got reverted. Now the merge request is again a draft, waiting for a new solution to the warnings problem.

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

No branches or pull requests

3 participants