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

Resolve target platform variables during p2 assembly (#3040) #3042

Closed

Conversation

kysmith-csg
Copy link
Contributor

This updates the p2-repository-plugin:assemble-repository mojo to resolve any variables in the target definition locations just as the target-platform-configuration is able to do.

I modified the existing tests for target variables to include this because they are related and should both work together.

Christoph Läubrich and others added 30 commits June 23, 2023 09:47
add maven 3.9 info
Bumps [maven-resolver-util](https://github.com/apache/maven-resolver) from 1.9.12 to 1.9.13.
- [Release notes](https://github.com/apache/maven-resolver/releases)
- [Commits](apache/maven-resolver@maven-resolver-1.9.12...maven-resolver-1.9.13)

---
updated-dependencies:
- dependency-name: org.apache.maven.resolver:maven-resolver-util
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps `maven-version` from 3.9.2 to 3.9.3.

Updates `maven-plugin-api` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `maven-core` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `maven-artifact` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `maven-compat` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `maven-model` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `maven-settings` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `maven-model-builder` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `apache-maven` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

Updates `maven-embedder` from 3.9.2 to 3.9.3
- [Release notes](https://github.com/apache/maven/releases)
- [Commits](apache/maven@maven-3.9.2...maven-3.9.3)

---
updated-dependencies:
- dependency-name: org.apache.maven:maven-plugin-api
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:maven-core
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:maven-artifact
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:maven-compat
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:maven-model
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:maven-settings
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:maven-model-builder
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:apache-maven:bin
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: org.apache.maven:maven-embedder
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
This character is illegal in Windows file paths.

(cherry picked from commit dbb779b)
This simplifies testing of comparators with different values of this
property.

(cherry picked from commit 5b5d2b1)
Bumps [unirest-java](https://github.com/Kong/unirest-java) from 3.14.4 to 3.14.5.
- [Release notes](https://github.com/Kong/unirest-java/releases)
- [Changelog](https://github.com/Kong/unirest-java/blob/main/CHANGELOG.md)
- [Commits](Kong/unirest-java@v3.14.4...v3.14.5)

---
updated-dependencies:
- dependency-name: com.konghq:unirest-java
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Don't ignore all kind of newlines when comparing text-files, only treat
exactly matching \n and \r\n as equals.

(cherry picked from commit 199647e)
Bumps depends-maven-plugin from 1.4.0 to 1.5.0.

---
updated-dependencies:
- dependency-name: org.apache.servicemix.tooling:depends-maven-plugin
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Currently the tests in the build are not executed for the junit
container, the testproject is only compiled because an execution is
missing.

This adds the missing execution and an assertion that ensures the tests
are run.

(cherry picked from commit d3aeec7)
Currently Tycho fails when the JUnit-Container is used but no JUnit is
in the target platform. Actually the JUnit Container itself implies
already some set of "target content" that should be supplied by Tycho.

(cherry picked from commit 1532e01)
Currently there needs to be a mojo for each format where the basics are
quite common and just the serialization is special so it seems quite
useful to reuse the basics.

(cherry picked from commit 374e2b2)
Bumps [org.apache.maven.resolver:maven-resolver-util](https://github.com/apache/maven-resolver) from 1.9.13 to 1.9.14.
- [Release notes](https://github.com/apache/maven-resolver/releases)
- [Commits](apache/maven-resolver@maven-resolver-1.9.13...maven-resolver-1.9.14)

---
updated-dependencies:
- dependency-name: org.apache.maven.resolver:maven-resolver-util
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [org.junit.vintage:junit-vintage-engine](https://github.com/junit-team/junit5) from 5.9.3 to 5.10.0.
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](junit-team/junit5@r5.9.3...r5.10.0)

---
updated-dependencies:
- dependency-name: org.junit.vintage:junit-vintage-engine
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
dependabot bot and others added 12 commits November 8, 2023 06:56
Bumps [org.junit.jupiter:junit-jupiter](https://github.com/junit-team/junit5) from 5.9.3 to 5.10.1.
- [Release notes](https://github.com/junit-team/junit5/releases)
- [Commits](junit-team/junit5@r5.9.3...r5.10.1)

---
updated-dependencies:
- dependency-name: org.junit.jupiter:junit-jupiter
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Actually each source.<jar> should have a corresponding output.<jar> but
there are some cases where this is not true... lets cheat and look at
the classpath instead...

Fix eclipse-tycho#3019
If there is only a single jar specified, Tycho uses the default output
folder instead of a special class folder.

This checks if there is only a single folder and then uses that one
instead.
Newer asm has been needed.
Fixes eclipse-tycho#2950

(cherry picked from commit bce5c53)
When using the javadoc goal to aggregate documentation using configured
source directories there is the problem that also a reference to
projects that are mentioned in the source directories is required.

This adds a new (configurable) injector that adds the projects mentioned
as the source as dependencies of the project.

(cherry picked from commit c5e6f22)
This adds a similar feature like PDE will offer soon see
eclipse-pde/eclipse.pde#912
Bumps [io.takari.polyglot:polyglot-common](https://github.com/takari/polyglot-maven) from 0.5.0 to 0.7.0.
- [Release notes](https://github.com/takari/polyglot-maven/releases)
- [Changelog](https://github.com/takari/polyglot-maven/blob/master/CHANGELOG.md)
- [Commits](takari/polyglot-maven@polyglot-0.5.0...polyglot-0.7.0)

---
updated-dependencies:
- dependency-name: io.takari.polyglot:polyglot-common
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Copy link

github-actions bot commented Nov 15, 2023

Test Results

   573 files  ±0     573 suites  ±0   3h 44m 18s ⏱️ + 12m 29s
   376 tests ±0     370 ✔️ ±0    6 💤 ±0  0 ±0 
1 128 runs  ±0  1 109 ✔️ ±0  19 💤 ±0  0 ±0 

Results for commit 428e238. ± Comparison against base commit c47ccf7.

♻️ This comment has been updated with latest results.

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

In general this looks good, just one suggestion below.

Furthermore it would be good to verify in the Test that the URL is properly resolved.
You could do it like in RepoRefLocationP2RepositoryIntegrationTest.

Is the plan to also apply this to Tycho 5? Because if you had opened the PR against the master it could have probably be back-ported automatically.

@kysmith-csg
Copy link
Contributor Author

Is the plan to also apply this to Tycho 5?

I guess so yeah. I'm just more used to fixing things the opposite way (lower branches first then merge-forward).

@HannesWell
Copy link
Member

Is the plan to also apply this to Tycho 5?

I guess so yeah. I'm just more used to fixing things the opposite way (lower branches first then merge-forward).

Understandable. But for back-ports from Tycho 5 to 4 an automated process exists where you just have to apply a lable and some GH actions do the rest.
AFAIK for the opposite way there is only the manual process.

Can you please also update the test case that runs the modified test-build to check for the resolved reference as indicated in my previous comment?
Then this change would be perfectly fine for me.

Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Pefect. Thanks.
Can you please squash all the there commits into one with a suitable Message/description for the overall change?

@kysmith-csg kysmith-csg force-pushed the issue_3040_reproducer branch from d609323 to 428e238 Compare November 16, 2023 20:41
@laeubi laeubi added the backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change label Nov 17, 2023
@laeubi
Copy link
Member

laeubi commented Nov 17, 2023

@kysmith-csg also please rebase this on top of master, Tycho 4 is more ore less in maintenance mode so we usually do not implement new features to Tycho 4 but rather implement in Tycho 5 branch (where we can change everything) and then decide if it is save to be backported or requires some adjustments (e.g. keeping a previous default), that way it can't happen that something is lost for Tycho 5.

@kysmith-csg
Copy link
Contributor Author

we usually do not implement new features to Tycho 4 but rather implement in Tycho 5 branch

I disagree that this is a new feature - it is most definitely a bug. But I agree that it should be on latest first then backported. I'm not super familiar with the system you guys have and didn't know they can be backported automatically. Maybe helpful to add to the contributing guide?

I'm not familiar at all with rebasing branches. I'll try to use the UI because seemingly I can change the branch. If it gets too messed up I'll just open a new PR.

@kysmith-csg kysmith-csg changed the base branch from tycho-4.0.x to master November 17, 2023 14:34
@kysmith-csg
Copy link
Contributor Author

Yeah. This is why we can't trust UIs :) remaking the PR...

@laeubi
Copy link
Member

laeubi commented Nov 17, 2023

I disagree that this is a new feature - it is most definitely a bug.

Whatever we classify it, we need it in both branches as the bug is present in both of them, we have an automated way to backport (compatible) thing from master > Tycho 4 branch so usually it is easier to show that the bug still exits on master and then let it automatically backport :-)

@HannesWell
Copy link
Member

Maybe helpful to add to the contributing guide?

I think so. Do you want to provide a PR?
There is already a backporting-section but that is more general

Yeah. This is why we can't trust UIs :) remaking the PR...

You probably would have to 'move' your commit to the master branch locally and force-push it. Moving can be done by switching to the master branch and cherry-picking the commit.
Any way, your approach worked as well 👍🏽

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change
Projects
None yet
Development

Successfully merging this pull request may close these issues.