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

fix for correct .m2 folder #716

Closed
wants to merge 2 commits into from

Conversation

tobka777
Copy link
Member

fixes #706

@tobka777 tobka777 self-assigned this Mar 15, 2022
@github-actions github-actions bot added bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) scripts related to shell scripts (bash and CMD) labels Mar 15, 2022
@tobka777 tobka777 added Team_IDE and removed scripts related to shell scripts (bash and CMD) bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) labels Mar 15, 2022
@github-actions github-actions bot added bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) scripts related to shell scripts (bash and CMD) labels Mar 15, 2022
@tobka777 tobka777 removed scripts related to shell scripts (bash and CMD) commandlet related to commandlets (scripts/command/*) labels Mar 15, 2022
@github-actions github-actions bot added commandlet related to commandlets (scripts/command/*) scripts related to shell scripts (bash and CMD) labels Mar 15, 2022
@@ -306,7 +306,7 @@ function doCheckNoSnapshots() {

function doRunBuild() {
local maven_cmd="${MVN}"
local maven_arguments=""
local maven_arguments="--settings=${DEVON_IDE_HOME}/conf/.m2/settings.xml"
Copy link
Member

@hohwille hohwille May 6, 2022

Choose a reason for hiding this comment

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

dont we need to export this to MAVEN_ARGS via devon.properties?
This only applies when mvn is called via devon as wrapper command.
However, I see this bug occurring all the time without devon command being involved as wrapper but only to setup env variables.
My assumption is that the actual bug #706 somehow originates from m2e eclipse plugin and happens since a newer version of that plugin so it did not happen before and suddenly came with some ide or eclipse update...

Copy link
Member

Choose a reason for hiding this comment

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

The current "hack" to relocate the settings to this directory happens here:
https://github.com/devonfw/ide/blob/master/scripts/src/main/resources/scripts/devon.properties#L19

So far this worked well for commandline (with and without devon wrapper) as well as for all supported IDEs.

Copy link
Member

Choose a reason for hiding this comment

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

later down the maven_arguments are overridden (maven_arguments="${MAVEN_ARGS}") so with maven 3.x your change will have no effect. As maven 4.x is still not released, this PR does not work or make a big difference.
Should we close this or should we change the later code in this PR to honor existing maven_arguments and append to it instead of overriding.

Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

@tobka777 thanks for this PR.
I am happy to approve it.
However, I am quite sure that it is not really related to the bug #706 and will not fix it.

Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

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

I just stumbled again over this PR when trying to review all open PRs and found a problem.
Once upon a time, you could rework or simply close this PR.

Comment on lines +322 to +323
maven_arguments="${MAVEN_ARGS}"
fi
Copy link
Member

Choose a reason for hiding this comment

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

did those changes happen by accident? Seems the indentation was messed by your editor somehow.

@@ -306,7 +306,7 @@ function doCheckNoSnapshots() {

function doRunBuild() {
local maven_cmd="${MVN}"
local maven_arguments=""
local maven_arguments="--settings=${DEVON_IDE_HOME}/conf/.m2/settings.xml"
Copy link
Member

Choose a reason for hiding this comment

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

later down the maven_arguments are overridden (maven_arguments="${MAVEN_ARGS}") so with maven 3.x your change will have no effect. As maven 4.x is still not released, this PR does not work or make a big difference.
Should we close this or should we change the later code in this PR to honor existing maven_arguments and append to it instead of overriding.

@hohwille hohwille added the waiting-for-feedback we have asked a question and the issue is pending until an answer is provided label Aug 29, 2022
@tobka777
Copy link
Member Author

@hohwille I think we can close the ticket for now. I think @alfeilex has identified the main problem with his fix.
#814

@tobka777 tobka777 closed this Sep 13, 2022
@hohwille hohwille added this to the rejected milestone Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) scripts related to shell scripts (bash and CMD) waiting-for-feedback we have asked a question and the issue is pending until an answer is provided
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Folder ${M2_REPO} gets created
3 participants