jmc
jmc copied to clipboard
7576: Remove the need for a local jetty server for dependencies
Using newer Tycho features to achieve result:
Maven dependencies in a target (Tycho 2.2 ): https://github.com/eclipse/tycho/blob/master/RELEASE_NOTES.md#support-for-m2e-pde-maven-target-locations Nested target to cut down on duplicate third party code (Tycho 2.6): https://github.com/eclipse/tycho/blob/master/RELEASE_NOTES.md#support-for-nested-targets
Note: Due to change in 2.7, you must delete a file when switching branches due to changes in the .m2 folder: https://github.com/eclipse/tycho/blob/master/RELEASE_NOTES.md#caution-when-switching-between-tycho-versions
Progress
- [x] Commit message must refer to an issue
- [ ] Change must be properly reviewed (1 review required, with at least 1 Committer)
Issue
- JMC-7576: Remove the need for a local jetty server for dependencies
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmc pull/387/head:pull/387
$ git checkout pull/387
Update a local copy of the PR:
$ git checkout pull/387
$ git pull https://git.openjdk.org/jmc pull/387/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 387
View PR using the GUI difftool:
$ git pr show -t 387
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jmc/pull/387.diff
Hi @abrooksv, welcome to this OpenJDK project and thanks for contributing!
We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.
If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user abrooksv" as summary for the issue.
If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.
/covered
Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!
Update for this PR: Eclipse can't resolve the third party dependencies in the manifest. This seems to be a known PDE issue: https://stackoverflow.com/questions/13728801/why-cant-manifest-mf-see-packages-from-mavens-pom
@abrooksv this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:
git checkout removeJetty
git fetch https://git.openjdk.org/jmc master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push
Hi @abrooksv! Is this something you'd like to get into JMC 9? Do you know if your changes will work well in the PDE / Eclipse?
@abrooksv This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
Hey @thegreystone, it was rough around the edges in Eclipse at the time, but PDE and Tycho were also a moving target in their improvements in the area.
I am not sure the state of it now since I have not used Eclipse in over a year
Hi Austin! Want to give this one last shot before JMC 9? We're in ramp down now, but I believe we could take this one if finished up.
@abrooksv Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.
Hey Marcus!
I started over and now it is happy inside of Eclipse. I only sanity tested the latest target so far but there is a small hitch. Now that Tycho is speaking Maven coords, it seems to dislike that we defined Jetty to be 10.0.17 while Eclipse Platform is shipping 10.0.15 which was upgraded in #532. It seems it wants to to match:
[ERROR] Failed to execute goal org.eclipse.tycho:target-platform-configuration:3.0.4:target-platform (default-target-platform) on project org.openjdk.jmc.feature.rcp: Execution default-target-platform of goal org.eclipse.tycho:target-platform-configuration:3.0.4:target-platform failed: The Maven artifact to be added to the target platform is not stored at the required location on disk: required "/Users/austin.brooks/.m2/repository/org/eclipse/jetty/jetty-io/10.0.15/jetty-io-10.0.15.jar" but was "/Users/austin.brooks/.m2/repository/org/eclipse/jetty/jetty-io/10.0.17/jetty-io-10.0.17.jar" -> [Help 1]
This limitation may cause issues to arise in the other platforms I assume, if so we would probably need to match the Jetty webcsocket version to the version of the Eclipse platform of the target platform (probably something that should have always been done to avoid risk of runtime error?)
Also means we will need to inline the third party libs again vs using a shared file, but that's just a return to the old way.
@abrooksv This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
@abrooksv This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.