uio-bmi / crypt4gh

Crypt4GH standard implementation
https://uio-bmi.github.io/crypt4gh/
MIT License
2 stars 6 forks source link

Resume publishing releases to maven central, please? #85

Open brainstorm opened 1 year ago

brainstorm commented 1 year ago

Is your feature request related to a problem? Please describe.

I'd like to incorporate this module on IGV desktop and the upstream maintainers might not accept the PR unless the .jar is publicly accessible (sans authenticated Github tokens). Unauthenticated access is planned but not implemented in GH yet:

https://github.com/orgs/community/discussions/26634

Describe the solution you'd like

It'd be very convenient to resume publishing here: https://mvnrepository.com/artifact/no.uio.ifi/crypt4gh

Github has detailed github actions workflows that allow parallel publishing (github + mavencentral).

Describe alternatives you've considered

I could just download the .jar and ship it with IGV's codebase, which is the approach I'll take for the time being, although it's suboptimal from a release and dependencies management perspective.

kjetilkl commented 1 year ago

Unfortunately, most of our team is on vacation at the moment, so it may take a few weeks before we have time to address this issue

brainstorm commented 1 year ago

Thanks @kjetilkl, I'll import it locally for now but it'd be excellent to have this back on Maven when your team is back, cheers!

/cc @mmalenic @reisingerf

brainstorm commented 1 year ago

By the way, the already published objects on Maven2 repo, don't seem to be properly published either:

Build file '/Users/rvalls/dev/umccr/igv/build.gradle' line: 476

A problem occurred evaluating root project 'igv'.
> Could not resolve all files for configuration ':default'.
   > Could not find no.uio.ifi:crypt4gh:2.4.2.
     Searched in the following locations:
       - https://repo.maven.apache.org/maven2/no/uio/ifi/crypt4gh/2.4.2/crypt4gh-2.4.2.pom
       - https://repo.maven.apache.org/maven2/no/uio/ifi/crypt4gh/2.4.2/crypt4gh-2.4.2.jar
     Required by:
         project :

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

If you go to the raw maven2 file listing, none of the objects are there: https://repo.maven.apache.org/maven2/no/

kjetilkl commented 1 year ago

Versions of crypt4gh up to 2.4.2 were published on JCentral, which was shut down in 2022. This is the reason why these old packages are no longer available and why we started publishing to GitHub Packages instead. But we have decided to start publishing to Maven Central now (or maybe publish in both places). We will also discuss your Pull Request next week.

brainstorm commented 10 months ago

Thanks @kjetilkl! We are putting the pieces together for support on the backend side of things for Crypt4GH and we were wondering how's that "return to Maven publishing" effort is going?

kjetilkl commented 10 months ago

I'm sorry that the progress was impeded for a long time by slow decision making on our part, but we have a sort of deadline next Wednesday to get this done now.

By the way, when I tried to compile the modularized code, I got a warning from Maven that worried me with regards to publishing:

[WARNING] * Required filename-based automodules detected: [blake2b-1.0.0.jar, bkdf-0.6.0.jar, bcrypt-0.10.2.jar, scrypt-1.4.0.jar]. Please don't publish this project to a public artifact repository! *
brainstorm commented 10 months ago

By the way, when I tried to compile the modularized code, I got a warning from Maven that worried me with regards to publishing

Indeed, that worried me a bit too. If you haven't already I'll try to determine what's the root cause of that and potentially fix it... maybe it requires modularising (modernize) those upstream modules yourself as well as crypt4gh itself 🤷🏻 ?

brainstorm commented 10 months ago

@kjetilkl upon second inspection I think this could be that dependencies are indeed not yet being adopted as modules? There's this second WARNING block a few lines below the filename-based automodules warning (and some of those package names overlap the ones from the prior warning you pointed out):

[WARNING] Discovered module-info.class. Shading will break its strong encapsulation.
[WARNING] hkdf-2.0.0.jar, slf4j-api-2.0.7.jar, slf4j-jdk14-2.0.7.jar define 1 overlapping classes: 
[WARNING]   - META-INF.versions.9.module-info
[WARNING] bcrypt-0.10.2.jar, bkdf-0.6.0.jar, blake2b-1.0.0.jar, bytes-1.6.1.jar, commons-cli-1.5.0.jar, commons-codec-1.16.0.jar, commons-io-2.13.0.jar, commons-lang3-3.13.0.jar, crypt4gh-2.4.8.jar, hkdf-2.0.0.jar, scrypt-1.4.0.jar, slf4j-api-2.0.7.jar, slf4j-jdk14-2.0.7.jar define 1 overlapping resource: 
[WARNING]   - META-INF/MANIFEST.MF
[WARNING] commons-cli-1.5.0.jar, commons-codec-1.16.0.jar, commons-io-2.13.0.jar, commons-lang3-3.13.0.jar define 1 overlapping resource: 
[WARNING]   - META-INF/NOTICE.txt
[WARNING] commons-cli-1.5.0.jar, commons-codec-1.16.0.jar, commons-io-2.13.0.jar, commons-lang3-3.13.0.jar, slf4j-api-2.0.7.jar, slf4j-jdk14-2.0.7.jar define 1 overlapping resource: 
[WARNING]   - META-INF/LICENSE.txt
[WARNING] maven-shade-plugin has detected that some files are
[WARNING] present in two or more JARs. When this happens, only one
[WARNING] single version of the file is copied to the uber jar.
[WARNING] Usually this is not harmful and you can skip these warnings,
[WARNING] otherwise try to manually exclude artifacts based on
[WARNING] mvn dependency:tree -Ddetail=true and the above output.
[WARNING] See https://maven.apache.org/plugins/maven-shade-plugin/

I'm unsure if this would really affect you much, but let's do this properly: Let me try to poke a bit upstream for those affected dependencies and their maintainers...

brainstorm commented 10 months ago

Alright, as you might have noticed by your GH notifications today, I've opened o couple of pullrequests to the upstream libraries. If those fail to respond we could just remap them under your GroupID namespace and push them to Maven anyway.

The last remaining one to fix, scrypt seems to be properly discontinued upstream (archived): https://github.com/wg/scrypt ... so for that one, what would you prefer:

  1. Following the approach I went for the aforementioned modules.
  2. Choose another fork that might be operational (see https://github.com/wg/scrypt/network).
  3. Choose an entirely new but probably maintained (and that doesn't have CVEs on its dependencies): https://mvnrepository.com/search?q=scrypt

Hope this all helps!

kjetilkl commented 9 months ago

After a long time, I have finally been able to create a snapshot release. It is based on the new_groupid branch where the groupID and package name has been changed from no.uio.ifi to no.elixir. I will make the final release once the issues with the unnamed modules have been resolved.

brainstorm commented 9 months ago

Awesome! Thanks @kjetilkl.

I'm skeptical that the other couple of packages remaining (those written by @patrickfav, namely bkdf and bcrypt) will be merged and released (unless he replies and accepts the changes like the blake2b author)... so instead of waiting (indefinitely?) I'd advise to just fork my PR (https://github.com/patrickfav/bkdf/pull/3) make the appropriate minor changes to publish it under your own groupid and release it. Let me know if you want help with that.

Same strategy would apply to the orphaned scrypt dependency, I reckon.

brainstorm commented 1 week ago

Hello @kjetilkl, I've noticed some recent activity on this repo so I'm touching base on this issue: if you need any help from me, happy to help push it to Maven if it's not already (other than the snapshot)!

kjetilkl commented 1 week ago

We have actually moved all of our own code (including Crypt4GH) into another Gradle-based monorepo now, so we had planned to archive this repository and not maintain it anymore. However, since our other projects no longer depend on this particular Crypt4GH, we have more freedom regarding what to do with it going forward, including where and how to publish it. For one, it does not have to be shaded anymore. Some assistance would be appreciated, since I'm swamped with work for at least the next month.