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 Build scan capture extension registration when using the Maven wrapper #37568

Merged
merged 1 commit into from Dec 11, 2023

Conversation

jprinet
Copy link
Contributor

@jprinet jprinet commented Dec 6, 2023

Issue

No Develocity Build scans are being published because the Maven extension to capture unpublished Build Scans is not registered.

When using the Maven wrapper, a Maven extension can't be registered once at the beginning of the build by copying it to $MAVEN_HOME/lib/ext as the folder is created on the first mvnw invocation (and emptied if already present).

Fix

Install the Maven extension in ./maven-build-scan-capture/lib/ext (using a relative path makes this approach compatible with Windows OS) and register the Maven extension as a Maven CLI argument (-Dmaven.ext.class.path)

@quarkus-bot quarkus-bot bot added the area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure label Dec 6, 2023
@@ -42,7 +42,7 @@ concurrency:
env:
# Workaround testsuite locale issue
LANG: en_US.UTF-8
COMMON_MAVEN_ARGS: "-e -B --settings .github/mvn-settings.xml --fail-at-end"
COMMON_MAVEN_ARGS: "-e -B --settings .github/mvn-settings.xml --fail-at-end -Dmaven.ext.class.path=maven-build-scan-capture/lib/ext/maven-build-scan-capture-extension.jar"
Copy link
Member

Choose a reason for hiding this comment

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

We cannot register the extension in the pom.xml directly using a profile? That would maybe makes things easier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a core extension and according to the doc, the registration options are:

  • $MAVEN_HOME/lib/ext
  • extensions.xml
  • CLI argument -Dmaven.ext.class.path=extension.jar

This extension should not be added systematically, therefore the CLI argument solution

@jprinet jprinet force-pushed the fix/build-scan-publication branch 6 times, most recently from 798ca68 to 8cd3cf6 Compare December 11, 2023 09:12
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Thanks!

@gsmet gsmet merged commit 5ae84a2 into quarkusio:main Dec 11, 2023
2 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Dec 11, 2023
@gsmet
Copy link
Member

gsmet commented Dec 11, 2023

@jprinet I was a bit too optimistic on this one and had to revert it because it looks as if build-scan-data are included in nested projects:

java.lang.AssertionError: 
[Snapshot is not matching (use -Dsnap to update it automatically):CreateExtensionMojoIT/testCreateCoreExtension/dir-tree.snapshot] 
Expecting actual:
  ["/",
    "bom/",
    "bom/application/",
    "bom/application/pom.xml",
    "build-scan-data/",
    "build-scan-data/1.18.1/",
    "build-scan-data/1.18.1/previous/",
    "build-scan-data/1.18.1/previous/1702291663404-27469121-9c20-4e47-aed6-32db6139e80e/",
    "build-scan-data/1.18.1/previous/1702291663404-27469121-9c20-4e47-aed6-32db6139e80e/build-scan-metadata.properties",
    "build-scan-data/1.18.1/previous/1702291663404-27469121-9c20-4e47-aed6-32db6139e80e/scan.properties",
    "build-scan-data/1.18.1/previous/1702291663404-27469121-9c20-4e47-aed6-32db6139e80e/scan.scan",
    "build-scan-data/1.18.1/spool/",
    "build-scan-data/1.18.1/spool/13885e8f-487d-4762-afa2-b5f81fb9904b.scan",
    "build-scan-data/1.18.1/spool/245faeed-0f66-4b51-9218-55afa36cad55.scan",
    "build-scan-data/1.18.1/spool/30044a69-f46f-42cd-bac9-a2ae8f1a930d.scan",
    "build-scan-data/1.18.1/spool/4d59400a-0049-40b8-ab5c-e09d3ac72236.scan",
    "build-scan-data/1.18.1/spool/5e84a84d-2431-4c39-bcce-de340fbda870.scan",
    "build-scan-data/1.18.1/spool/9e3a4921-24cc-4457-a2eb-6e3dc92d014c.scan",
    "build-scan-data/1.18.1/spool/b86dce74-01d9-4a27-ab87-43656eb112bd.scan",
    "build-scan-data/1.18.1/spool/c54024a9-6f1d-4cb1-a39a-5af77ea6f476.scan",
    "build-scan-data/1.18.1/spool/c94d5aaf-63a4-486b-aa7b-863c359438c4.scan",
    "extensions/",

@gsmet
Copy link
Member

gsmet commented Dec 11, 2023

Oh wait, no, there's something very odd with this test...

@gsmet
Copy link
Member

gsmet commented Dec 11, 2023

OK, so the test is adding an extension in the tree itself and then testing that the tree doesn't contain anything odd.

I think the build-scan-data directory should be moved to target/ if possible to avoid anything odd?

@jprinet
Copy link
Contributor Author

jprinet commented Dec 11, 2023

I changed the copy directory to env.RUNNER_TEMP
I validated the behavior on Windows and Linux runners
Can you give it a try @gsmet ?

@jprinet
Copy link
Contributor Author

jprinet commented Dec 11, 2023

Alright, you reverted the change, let me submit a new PR

@jprinet
Copy link
Contributor Author

jprinet commented Dec 11, 2023

I created #37663 @gsmet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants