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

POM is not preserved when we add or remove extensions with CLI #40853

Closed
jcarranzan opened this issue May 27, 2024 · 12 comments · Fixed by #40869
Closed

POM is not preserved when we add or remove extensions with CLI #40853

jcarranzan opened this issue May 27, 2024 · 12 comments · Fixed by #40869
Assignees
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) kind/bug Something isn't working
Milestone

Comments

@jcarranzan
Copy link
Contributor

Describe the bug

I was testing the backport related to preserving POM integrity after adding an extension ( 2bb40f9).
However, it seems that POM integrity is still not preserved when we add or remove extensions through CLI commands.
I've created a reproducer to show you this behavior (git clone -b pom-preserve-cli https://github.com/jcarranzan/quarkus-reproducer.git).

Expected behavior

The POM retains the same structure and comments after adding or removing an extension.

Actual behavior

The POM is overwritten, and comments are not preserved.

How to Reproduce?

git clone -b pom-preserve-cli https://github.com/jcarranzan/quarkus-reproducer.git

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

3.8.999-SNAPSHOT

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@jcarranzan jcarranzan added the kind/bug Something isn't working label May 27, 2024
@quarkus-bot quarkus-bot bot added the area/cli Related to quarkus cli (not maven/gradle/etc.) label May 27, 2024
Copy link

quarkus-bot bot commented May 27, 2024

/cc @ebullient (cli), @maxandersen (cli)

@gsmet
Copy link
Member

gsmet commented May 27, 2024

Which command line did you run, exactly? Wondering if you were using the correct version of the plugin, as I think you might have to specify it on the command line.

@rsvoboda
Copy link
Member

@rsvoboda
Copy link
Member

@jcarranzan mvn clean verify -Dquarkus.platform.version=3.10.2 fails too

I don't think you persist comment addition back to the pom.xml file after https://github.com/jcarranzan/quarkus-reproducer/blob/pom-preserve-cli/src/test/java/io/quarkus/QuarkusCliPomIntegrityIT.java#L39C34-L39C41

@gsmet
Copy link
Member

gsmet commented May 27, 2024

I would recommend to start with a POM file containing comments and new lines.

@jcarranzan
Copy link
Contributor Author

jcarranzan commented May 27, 2024

I tweak the test (https://github.com/jcarranzan/quarkus-reproducer/blob/pom-preserve-cli/src/test/java/io/quarkus/QuarkusCliPomIntegrityIT.java) with the @rsvoboda suggestion,

  1. It creates a Quarkus application named APP_NAME using the QuarkusCliClient.
  2. Reads the initial content of the POM file.
  3. Adds the COMMENT at line 1, and writes the modified content back to the file.
  4. Next, it installs the new extension usingapp.installExtension()with QuarkusCliClient from QuarkusCliRestService .
  5. After installation, it reads the updated content of the POM file and asserts that it contains the comment added.
    Then the test fails because the comment is not there.

@rsvoboda
Copy link
Member

rsvoboda commented May 28, 2024

Thanks @jcarranzan for the update.

I tried with quarkus command (version 3.10.2) and I can confirm that the comments are not preserved @gsmet

quarkus create app foo:bat:1.0
cd bat
# open pom.xml, add comment to the file, save
quarkus ext add quarkus-kafka-client
# check pom.xml, comment is not there

@rsvoboda
Copy link
Member

@gastaldi can you look into this? (As you worked on #39382)

@gastaldi gastaldi self-assigned this May 28, 2024
@gastaldi
Copy link
Contributor

I've investigated and figured out that's caused by 68a1bbe, I'm working on a fix right now.

gastaldi added a commit to gastaldi/quarkus that referenced this issue May 28, 2024
This is necessary so `maven-model-helper` can know which pom.xml the change refers to.

- Fixes quarkusio#40853
@gastaldi
Copy link
Contributor

gastaldi commented May 28, 2024

@jcarranzan I've tested with your reproducer with the latest Quarkus CLI (using jbang app install --force --name quarkus ~/.m2/repository/io/quarkus/quarkus-cli/999-SNAPSHOT/quarkus-cli-999-SNAPSHOT-runner.jar)
and it works with my patch if you add the comment inside the pom.xml (eg. initialPomContent.add(3, COMMENT);).

@gastaldi gastaldi changed the title POM is not preserve when we adding or removing extensions with CLI POM is not preserved when we adding or removing extensions with CLI May 28, 2024
@gastaldi gastaldi changed the title POM is not preserved when we adding or removing extensions with CLI POM is not preserved when we add or remove extensions with CLI May 28, 2024
@jcarranzan
Copy link
Contributor Author

Ok, thank you @gastaldi.
I've tried it but still not worked, I guess just because I don't have your patch yet merged in the branch...

@gastaldi
Copy link
Contributor

@jcarranzan yes, you need to build my branch to test that

gastaldi added a commit to gastaldi/quarkus that referenced this issue May 29, 2024
This is necessary so `maven-model-helper` can know which pom.xml the change refers to.

- Fixes quarkusio#40853
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 30, 2024
@gsmet gsmet modified the milestones: 3.12 - main, 3.8.5 Jun 4, 2024
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jun 4, 2024
This is necessary so `maven-model-helper` can know which pom.xml the change refers to.

- Fixes quarkusio#40853

(cherry picked from commit 41ae1b3)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jun 4, 2024
This is necessary so `maven-model-helper` can know which pom.xml the change refers to.

- Fixes quarkusio#40853

(cherry picked from commit 41ae1b3)
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jun 4, 2024
This is necessary so `maven-model-helper` can know which pom.xml the change refers to.

- Fixes quarkusio#40853

(cherry picked from commit 41ae1b3)
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Jul 31, 2024
This is necessary so `maven-model-helper` can know which pom.xml the change refers to.

- Fixes quarkusio#40853
danielsoro pushed a commit to danielsoro/quarkus that referenced this issue Sep 20, 2024
This is necessary so `maven-model-helper` can know which pom.xml the change refers to.

- Fixes quarkusio#40853
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to quarkus cli (not maven/gradle/etc.) kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants