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

Download Maven dependencies on bootstrap #1135

Closed

Conversation

tiagobento
Copy link
Contributor

Bootstrap will take a little longer, but the builds will fail faster if something is wrong. The total build time won't change.

@tiagobento tiagobento force-pushed the download-mvn-dependencies-on-bootstrap branch from 4d48590 to a73d77f Compare July 28, 2022 18:43
@tiagobento
Copy link
Contributor Author

Hmm... looks like mvn dependency:resolve is not able to detect local dependencies that are not built yet? I can't help but think that this is maybe caused by a wrong configuration from our side. @yesamer Would you know?

@tiagobento tiagobento requested a review from yesamer July 28, 2022 18:54
Copy link
Contributor

@yesamer yesamer left a comment

Choose a reason for hiding this comment

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

@tiagobento For your goal, I think it's better to use mvn dependency:go-offline. With this command, you will download only the external dependencies required to build the project even if you are offline. I tested it in stunned-editor and works well. The only prerequisite is to use a recent version of maven-dependency-plugin.
mvn dependency:resolve tries to resolve internal modules dependency too, that are not available in external repo or previously completed and installed.

Apologies if I directly modified your PR, my first intent was to suggest changes. However, it was useful to see the effect of my changes

EDIT: In dashbuider there's this dependency io.netty:netty-transport-native-unix-common:jar:${os.detected.name}-${os.detected.arch}:4.1.59.Final which is not able to find - investigating

@yesamer
Copy link
Contributor

yesamer commented Jul 29, 2022

Things are getting interesting netty/netty#11272

@tiagobento
Copy link
Contributor Author

@yesamer Hey! Wow, thanks for really going for it :D No worries about committing directly here, actually, I think that's great 😄

And about the error on the netty dependency.. maybe we don't need that anymore? cc @jesuino

Also, an interesting question... how does it work currently? 🤔

@ederign
Copy link
Contributor

ederign commented Jul 29, 2022

@tiagobento where the netty is used?

@yesamer
Copy link
Contributor

yesamer commented Jul 29, 2022

@tiagobento As I wrote you in chat, netty is not actually used according to mvn dependency:analyzer
Imported but not used.

@yesamer
Copy link
Contributor

yesamer commented Jul 29, 2022

Found a usage import io.netty.util.internal.shaded.org.jctools.queues.MessagePassingQueue.Consumer @ DashboardExporter is maybe required for that?

@caponetto
Copy link
Contributor

caponetto commented Jul 29, 2022

Found a usage import io.netty.util.internal.shaded.org.jctools.queues.MessagePassingQueue.Consumer @ DashboardExporter is maybe required for that?

Maybe this Consumer should be java.util.function.Consumer.

@ederign ederign closed this Jul 29, 2022
@ederign ederign reopened this Jul 29, 2022
@tiagobento
Copy link
Contributor Author

Ok, so it looks like everything is working great ok Windows, but there's a weird failure on Ubuntu and macOS... very odd scenario 😝

Copy link
Contributor

@ederign ederign left a comment

Choose a reason for hiding this comment

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

LGTM. +1 to merge when build is green

@tiagobento
Copy link
Contributor Author

@ederign Thx!! @yesamer and I spoke today about this PR, and we agreed that we need to investigate a little bit more to see why it failed downloading dependencies for the Java Completion Plugin.. although it passed now, it doesn't seem stable enough for us to merge it.

We're going to try and find the root cause of those failures first.

@tiagobento
Copy link
Contributor Author

This is not stable enough to go in. Closing for now. Thanks @yesamer for all the help investigating this!

@tiagobento tiagobento closed this Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants