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

Bump jzlib from 1.1.3 to 1.1.3-kohsuke-1 #263

Merged
merged 1 commit into from Aug 17, 2021

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Aug 16, 2021

Bumps jzlib from 1.1.3 to 1.1.3-kohsuke-1.

Commits
  • ef08d09 [maven-release-plugin] prepare release jzlib-1.1.3-kohsuke-1
  • 2cc37d5 Actually, this is based on 1.1.3
  • 7204d0e [maven-release-plugin] prepare for next development iteration
  • 0246cda [maven-release-plugin] prepare release jzlib-1.1.2-kohsuke-1
  • 5ebc2a6 Preparing for a patch release
  • 3e3dae9 improving error diagnosis.
  • See full diff in compare view

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot added dependencies java Pull requests that update Java code labels Aug 16, 2021
@dependabot dependabot bot force-pushed the dependabot/maven/com.jcraft-jzlib-1.1.3-kohsuke-1 branch 2 times, most recently from 5f7adf4 to db88da0 Compare August 16, 2021 19:07
@timja
Copy link
Member

timja commented Aug 16, 2021

@basil @jglick any thoughts?

KK upstreamed this change: ymnk/jzlib#11 but it was never released

@jglick
Copy link
Member

jglick commented Aug 16, 2021

Yeah the library seems to have been abandoned, so we should not compound the problem by using a fork.

IIRC @jtnord suggested dispensing with the org.kohsuke.stapler.compression package. I see it used in e.g. https://github.com/jenkinsci/jenkins/blob/bd7c823c2781d9282eab71a5e2967d386c629344/core/src/main/java/hudson/model/Api.java#L194-L195 and https://github.com/jenkinsci/jenkins/blob/f6378676b11288cc2525852b0a1a441d147e31d3/core/src/main/java/hudson/model/LargeText.java#L214-L219 but I think it is also used for general-purpose page rendering? We could use https://github.com/eclipse/jetty.project/tree/jetty-9.4.x/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip instead. Or we could switch to another gzip library, or another encoding altogether. https://en.wikipedia.org/wiki/HTTP_compression#Content-Encoding_tokens suggests that we could use deflate with Java Platform calls alone, or Brotli with whatever library. Or we could just remove compression under the assumption that most clients are collocated with the Jenkins service on a fast network.

@timja
Copy link
Member

timja commented Aug 16, 2021

#265

@timja timja closed this Aug 16, 2021
@dependabot @github
Copy link
Contributor Author

dependabot bot commented on behalf of github Aug 16, 2021

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

@timja timja deleted the dependabot/maven/com.jcraft-jzlib-1.1.3-kohsuke-1 branch August 16, 2021 19:53
@jtnord
Copy link
Member

jtnord commented Aug 16, 2021

Oh.. now this is from memory..

Compress requires buffering, or chunked encoding (because you don't know the content length until after the compression has occurred).

We don't even need to compress ourselves IIRC we could use jetty filter / Handler to do it in winstone, or let whatever we app container configure it.

Support is now common for this I think we no longer need to roll out own.

https://www.eclipse.org/jetty/javadoc/jetty-9/org/eclipse/jetty/server/handler/gzip/GzipHandler.html

@basil
Copy link
Member

basil commented Aug 17, 2021

@basil […] any thoughts?

Of note is that Jenkins core (the only real consumer of Stapler) is already using 1.1.3-kohsuke-1. From core/pom.xml:

    <!-- Overriding Stapler’s 1.1.3 version to diagnose JENKINS-20618: -->
    <dependency>
      <groupId>com.jcraft</groupId>
      <artifactId>jzlib</artifactId>
    </dependency>
    <dependency>

The version is defined in bom/pom.xml as 1.1.3-kohsuke-1.

Based on the above, I see no reason not to take the Dependabot update and align the version in Stapler with what we actually ship in Jenkins core. It is effectively a no-op from the perspective of the shipped WAR but makes Dependabot happy.

Of course I agree that in the long term eliminating our dependency on this unmaintained library is a good idea, but that is a larger project.

@timja timja restored the dependabot/maven/com.jcraft-jzlib-1.1.3-kohsuke-1 branch August 17, 2021 16:13
@timja timja reopened this Aug 17, 2021
@timja timja enabled auto-merge (squash) August 17, 2021 16:13
Bumps [jzlib](https://github.com/kohsuke/jzlib) from 1.1.3 to 1.1.3-kohsuke-1.
- [Release notes](https://github.com/kohsuke/jzlib/releases)
- [Changelog](https://github.com/kohsuke/jzlib/blob/master/ChangeLog)
- [Commits](kohsuke/jzlib@1.1.3...jzlib-1.1.3-kohsuke-1)

---
updated-dependencies:
- dependency-name: com.jcraft:jzlib
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/maven/com.jcraft-jzlib-1.1.3-kohsuke-1 branch from db88da0 to b4c3b98 Compare August 17, 2021 16:13
@timja timja merged commit d25a30c into master Aug 17, 2021
@timja timja deleted the dependabot/maven/com.jcraft-jzlib-1.1.3-kohsuke-1 branch August 17, 2021 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants