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

Resume publishing releases to maven central, please? #85

Open
brainstorm opened this issue Jul 21, 2023 · 15 comments
Open

Resume publishing releases to maven central, please? #85

brainstorm opened this issue Jul 21, 2023 · 15 comments
Assignees

Comments

@brainstorm
Copy link
Contributor

brainstorm commented Jul 21, 2023

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.

brainstorm added a commit to umccr/igv that referenced this issue Jul 21, 2023
…help with the github actions workflow for maven if you need
@kjetilkl kjetilkl self-assigned this Jul 26, 2023
@kjetilkl
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Collaborator

kjetilkl commented Nov 9, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

brainstorm commented Nov 13, 2023

@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
Copy link
Contributor Author

brainstorm commented Nov 13, 2023

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
Copy link
Collaborator

kjetilkl commented Dec 5, 2023

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
Copy link
Contributor Author

brainstorm commented Dec 6, 2023

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 (patrickfav/bkdf#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
Copy link
Contributor Author

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
Copy link
Collaborator

kjetilkl commented Sep 6, 2024

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.

@brainstorm
Copy link
Contributor Author

We have actually moved all of our own code (including Crypt4GH) into another Gradle-based monorepo now

Interesting! Can you point me to that crypt4gh-related monorepo? I'd like to take a peek before putting efforts in this one?

@kjetilkl
Copy link
Collaborator

https://github.com/ELIXIR-NO/FEGA-Norway

Crypt4GH is under lib/crypt4gh

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

2 participants