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

Upgrade Gradle and jpi plugin #1216

Merged
merged 4 commits into from
Mar 17, 2020
Merged

Conversation

sghill
Copy link
Contributor

@sghill sghill commented Feb 19, 2020

Hello,

We're currently working on 0.39.0 of gradle-jpi-plugin, which requires Gradle 6, makes various improvements to dependency resolution, and supports Gradle Module Metadata.

This PR doesn't quite get all the way there because this plugin cannot currently be upgraded to Gradle 6. The job-dsl-api-viewer project uses a plugin that is currently incompatible with Gradle 6: eriwen/gradle-css-plugin#58. Still, it should minimize that change when possible.

Upgrade Gradle from 5.0 -> 5.6.4, the last version in the 5.x series. Changes to gradlew and gradlew.bat were caused by running ./gradlew wrapper. Release notes for intermediate versions are available here:

This change also updates the gradle-jpi-plugin from 0.28.1 -> 0.38.0, the last version to support Gradle 5.x. This change includes upgrading the jenkins-test-harness dependency from 2.31 -> 2.60. As of 2.45, the default behavior of temp directories changed to include a space, which broke many tests. The system property jenkins.test.noSpaceInTmpDirs changes this behavior back and allows the tests to pass.

@sghill
Copy link
Contributor Author

sghill commented Mar 5, 2020

Hi @daspilker (most recent committer), I see a few tests are failing only on Windows with:

java.nio.file.AccessDeniedException

Before I dive more into finding out what's happening, is this sort of change likely to be merged if the build passes?

@daspilker
Copy link
Member

Hi @sghill, I was actually waiting for someone to do the update 😄

I tested your changes on my Windows machine and the build was successful. Unfortunately the CI is a bit flaky and currently https://ci.jenkins.io seems to be down. It would be great if you can find out what's the problem.

@sghill sghill force-pushed the upgrade-jpi-plugin branch 4 times, most recently from b805350 to cea9a2a Compare March 7, 2020 16:45
@daspilker
Copy link
Member

@sghill I think the problem is that the Jenkins Test Harness fails to delete it's Jenkins home directory after the test. We would need a full stacktrace to verify that.

Another option would be to try to update Jenkins Test Harness or Jenkins Core to see if the problem has been fixed. On master I updated Jenkins core to 2.164. Can you rebase to master to pickup the changes? I'm also fine with updating core to a newer version if that helps.

@sghill
Copy link
Contributor Author

sghill commented Mar 11, 2020

Hi @daspilker,

I think the problem is that the Jenkins Test Harness fails to delete it's Jenkins home directory after the test. We would need a full stacktrace to verify that.

I can verify that. I've enabled full stacktrace:

javaposse.jobdsl.plugin.ExecuteDslScriptsSpec > user content is created FAILED

[2020-03-07T16:54:45.453Z]     jenkins.util.io.CompositeIOException: Unable to delete 'C:\Users\jenkins\AppData\Local\Temp\jkh1878956488277645840'. Tried 3 times (of a maximum of 3) waiting 0.1 sec between attempts.

I also added some logging to print out the file owner. I notice we're running as jenkins, but the workspace is owned by BUILTIN\Administrators (Alias). Who owns the workspace when the build succeeds on your Windows machine?

Can you rebase to master to pickup the changes?

This is done, but unfortunately it still failed.

@daspilker
Copy link
Member

daspilker commented Mar 13, 2020

@sghill I don't think that the file permissions are a problem. I think some files are still in use and thus can't be deleted on Windows.

One test causes a problem because a stream is not closed. I fixed that in b80273c.

I think the other tests fail because something (job executor?) is running while the tests finishes. As an indicator I got this exception:

javaposse.jobdsl.plugin.JenkinsJobManagementSpec > queue job without build FAILED
    java.nio.file.AccessDeniedException: C:\Users\Daniel\AppData\Local\Temp\jkh6234824681549231511\jobs\project\lastStable
        at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:83)
        at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
        at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
        at sun.nio.fs.WindowsLinkSupport.getFinalPath(WindowsLinkSupport.java:74)
        at sun.nio.fs.WindowsLinkSupport.getFinalPath(WindowsLinkSupport.java:111)
        at sun.nio.fs.WindowsAclFileAttributeView.getOwner(WindowsAclFileAttributeView.java:120)
        at sun.nio.fs.FileOwnerAttributeViewImpl.getOwner(FileOwnerAttributeViewImpl.java:91)
        at java.nio.file.Files.getOwner(Files.java:2079)
        at javaposse.jobdsl.plugin.FileOwnerPrinter$1.visitFile(FileOwnerPrinter.groovy:23)
        at java.nio.file.Files.walkFileTree(Files.java:2670)
        at java.nio.file.Files.walkFileTree(Files.java:2742)
        at javaposse.jobdsl.plugin.FileOwnerPrinter.print(FileOwnerPrinter.groovy:20)
        at javaposse.jobdsl.plugin.JenkinsJobManagementSpec.queue job without build(JenkinsJobManagementSpec.groovy:923)

Something is deleting files while the tree walker is running. I'm trying to fix the test.

@sghill
Copy link
Contributor Author

sghill commented Mar 13, 2020

This looks great @daspilker, the build is green!

Happy to remove the file owner printing debugging code I added unless you'd prefer to leave it in. I think the full stacktrace addition is probably useful in any case, but if you'd prefer that were another PR I can move that out too.

This also upgrades jenkins-test-harness 2.31 -> 2.60. Since 2.45, temp
directories are generated with spaces in the name by default. This
space behavior breaks existing tests and must be disabled by setting
the system property `jenkins.test.noSpaceInTmpDirs`.

See also:
- https://github.com/jenkinsci/jenkins-test-harness#245-2019-jan-10
- https://issues.jenkins-ci.org/browse/JENKINS-35638
@daspilker
Copy link
Member

I moved the fixes for the tests to separate PRs (#1222, #1223) and removed any commits unrelated to updating the JPI plugin and Gradle from this PR. I'll merge this when the CI is green.

@daspilker
Copy link
Member

😞

@daspilker daspilker merged commit 2468435 into jenkinsci:master Mar 17, 2020
@sghill sghill deleted the upgrade-jpi-plugin branch March 21, 2020 15:55
@sghill
Copy link
Contributor Author

sghill commented Mar 21, 2020

Thanks for merging this @daspilker! I appreciate the Windows debugging effort you put in and the test robustness improvements.

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