Skip to content
New issue

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

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

Already on GitHub? # to your account

[JENKINS-74977] Require Jenkins 2.452.4, depend on commons compress API plugin #30

Merged

Conversation

MarkEWaite
Copy link
Contributor

@MarkEWaite MarkEWaite commented Dec 10, 2024

JENKINS-74977 Require Jenkins 2.452.4, depend on commons compress API plugin

Jenkins 2.489 removes the commons compress library from Jenkins core. This plugin depended on the commons compress library being provided by Jenkins core in order to satisfy the transitive dependency from truezip-driver-zip. Make the plugin depend on commons-compress-api plugin instead of commons-core from Jenkins core so that the plugin can run on Jenkins versions 2.489 and later.

Jenkins 2.452.4 was selected as the minimum Jenkins version because:

This issue also blocks the update of the Jenkins acceptance test harness to use Jenkins 2.489. The pull request that fails due to the compress artifacts plugin is:

Also updates the Jenkinsfile to test with Java 17 and Java 21, since Jenkins core releases no longer support Java 11.

Supersedes:

Would very much appreciate a rapid merge and release of this change so that the acceptance test harness can pass again.

Testing done

Confirmed that the 109.v98a_db_d3cb_72c release of the plugin fails when loaded into Jenkins 2.489 after enabling Artifact Management for Builds with the "Compress Artifacts" setting. The Pipeline looks like this:

pipeline {
    agent any
    stages {
	stage('Archive-datefile') {
	    steps {
		sh 'date >> datefile.txt'
		archiveArtifacts artifacts: 'datefile.txt'
	    }
	}
    }
}

Confirmed that the same Pipeline works correctly with the updated version of the plugin from this change.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Jenkins 2.489 removes the commons compress library from Jenkins core.
This plugin depended on the commons compress library being provided
by Jenkins core in order to satisfy the transitive dependency from
truezip-driver-zip.  Make the plugin depend on commons-compress-api
plugin instead of commons-core from Jenkins core so that the plugin can
run on Jenkins versions 2.489 and later.

This issue also blocks the update of the Jenkins acceptance test harness
to use Jenkins 2.489.  The pull request that fails due to the compress
artifacts plugin is:

* jenkinsci/acceptance-test-harness#1860

Testing done:

Confirmed that the released plugin fails when loaded into Jenkins
2.489 after enabling Artifact Management for Builds with the "Compress
Artifacts" setting.  The Pipeline job looks like this:

pipeline {
    agent any
    stages {
        stage('Hello') {
            steps {
                sh 'date >> datefile.txt'
                archiveArtifacts artifacts: 'datefile.txt'
            }
        }
    }
}

Confirmed that the same Pipeline library works correctly with the updated
version of the plugin from this change.
@timja timja requested a review from a team December 10, 2024 21:54
@MarkEWaite MarkEWaite changed the title Require Jenkins 2.452.4, depend on commons compress API plugin [JENKINS-74977] Require Jenkins 2.452.4, depend on commons compress API plugin Dec 10, 2024
assertArrayEquals(new String[0], zs.list());
assertArrayEquals(new VirtualFile[0], zs.list("*"));
} finally {
compressor.stop();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The automated test that calls compressor.start() is now modified to never call compressor.stop() because Java 8 and later have modified the Thread.stop() method to always throw an exception when it is called. An Oracle Tecnote describes the details. I took the simple approach and accepted that if the thread were leaked from a test, it would be a very short-lived leak because the test process would end very soon.

MarkEWaite added a commit to MarkEWaite/acceptance-test-harness that referenced this pull request Dec 10, 2024
jenkinsci/compress-artifacts-plugin#30 is the
pull request that generated this incremental build.

jenkinsci#1860 is the
pull request that discovered the issue.

jenkinsci/jenkins#9958 is the pull request that
removed commons compress from Jenkins weekly 2.489.
@MarkEWaite
Copy link
Contributor Author

The acceptance test harness with the incremental build passes the 3 tests that were previously failing, but fails a different test (SnippetGeneratorTest). More investigation needed.

I'm unable to duplicate that failure on my local installation.

@MarkEWaite
Copy link
Contributor Author

@jglick could you approve and merge this pull request in order to resolve JENKINS-74977 and allow the acceptance test harness to use Jenkins 2.479? You're the only person in the @jenkinsci/compress-artifacts-plugin-developers group.

If you prefer, I am also willing to submit an adoption request so that I can merge the pull request. CD is configured, so labeling this as a "bug" and merging it should be enough.

@jonesbusy
Copy link

Thanks!

@timja timja added the bug label Dec 11, 2024
@timja timja merged commit d4aa6dd into jenkinsci:master Dec 11, 2024
18 checks passed
@MarkEWaite MarkEWaite deleted the depend-on-commons-compress-api-plugin branch December 11, 2024 12:04
@MarkEWaite
Copy link
Contributor Author

Thanks for reviewing @jonesbusy and thanks for merging @timja! The acceptance test harness pull request for 2.489 has merged:

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

Successfully merging this pull request may close these issues.

3 participants