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

Use a released version of maven-hpi-plugin #3205

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amuniz
Copy link
Member

@amuniz amuniz commented May 15, 2024

This is using an rc coming from
jenkinsci/maven-hpi-plugin#464 which fails locally when there are no github credentials.

If jenkinsci/maven-hpi-plugin#464 is really needed it should be released (probably behind a flag).

Testing done

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
    Options
  2. Ensure that the pull request title represents the desired changelog entry
    Options
  3. Please describe what you did
    Options
  4. Link to relevant issues in GitHub or Jira
    Options
  5. Link to relevant pull requests, esp. upstream and downstream changes
    Options
  6. Ensure you have provided tests - that demonstrates feature works or fixes the issue
    Options

This is using an rc coming from
jenkinsci/maven-hpi-plugin#464 which fails
locally when there are no github credentials.

If jenkinsci#464 is really needed it should be released (probably behind a flag).
@amuniz amuniz requested a review from a team as a code owner May 15, 2024 09:41
@amuniz amuniz requested a review from basil May 15, 2024 09:41
@amuniz
Copy link
Member Author

amuniz commented May 15, 2024

Asking @basil for a review, as author of jenkinsci/maven-hpi-plugin#464

@basil
Copy link
Member

basil commented May 15, 2024

which fails locally when there are no github credentials

It is guarded under the CI environment variable which should not be set when running locally

@jglick
Copy link
Member

jglick commented May 15, 2024

It is not about GH credentials but about including !incrementals in <mirrorOf> in settings.xml, if that was the issue.

From a quick glance at jenkinsci/maven-hpi-plugin#464 it seems it could be released as is perhaps with just a note that the new property is experimental and could be removed without notice?

@basil
Copy link
Member

basil commented May 15, 2024

But the CI environment variable should never be set locally, so that line of code should never be executed locally

@amuniz
Copy link
Member Author

amuniz commented May 15, 2024

But the CI environment variable should never be set locally

Well, I wanted to test a modification in the code which is supposed to run in CI, so I had to enable the CI flag.
Anyways, I don't think using an incremental release of the plugin based on an old revision is a good idea.

@basil
Copy link
Member

basil commented May 15, 2024

OK, even in that case, you must have something weird about your Maven mirror configuration, as it works fine for me locally.

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Requesting changes for the following reason: breaks CI build

In the most extreme case, I would vote for this code to be deleted rather than released, as this experiment has outlived its usefulness.

@jglick
Copy link
Member

jglick commented May 17, 2024

I have no knowledge of the status of the Launchable integration, but I guess

I would vote for this code to be deleted rather than released, as this experiment has outlived its usefulness.

would mean (too big to “suggest changes”) deleting

bom/prep.sh

Line 33 in a25bab9

# TODO https://github.com/jenkinsci/maven-hpi-plugin/pull/464

bom/prep.sh

Lines 41 to 42 in a25bab9

-DcommitHashes=target/commit-hashes.txt
mv sample-plugin/target/commit-hashes.txt "target/commit-hashes-${LINE}.txt"
amending

bom/prep.sh

Line 46 in a25bab9

# produces: target/{commit-hashes-*.txt,plugins.txt,lines.txt}
deleting

bom/Jenkinsfile

Lines 70 to 79 in 53343ba

withCredentials([string(credentialsId: 'launchable-jenkins-bom', variable: 'LAUNCHABLE_TOKEN')]) {
lines.each { line ->
def commitHashes = readFile "commit-hashes-${line}.txt"
sh "launchable verify && launchable record build --name ${env.BUILD_TAG}-${line} --no-commit-collection " + commitHashes
def sessionFile = "launchable-session-${line}.txt"
sh "launchable record session --build ${env.BUILD_TAG}-${line} --flavor platform=linux --flavor jdk=17 >${sessionFile}"
stash name: sessionFile, includes: sessionFile
}
}

bom/Jenkinsfile

Lines 109 to 114 in 53343ba

withCredentials([string(credentialsId: 'launchable-jenkins-bom', variable: 'LAUNCHABLE_TOKEN')]) {
def sessionFile = "launchable-session-${line}.txt"
unstash sessionFile
def session = readFile(sessionFile).trim()
sh "launchable verify && launchable record tests --session ${session} --group ${repository} maven './**/target/surefire-reports' './**/target/failsafe-reports'"
}
right?

@basil
Copy link
Member

basil commented May 17, 2024

Well, I wanted to test a modification in the code which is supposed to run in CI, so I had to enable the CI flag.

What is this all about?

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

Successfully merging this pull request may close these issues.

None yet

3 participants