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 Orbit version of ASM that is shared by other projects using Orbit #257

Merged
merged 1 commit into from May 10, 2022

Conversation

merks
Copy link
Contributor

@merks merks commented May 6, 2022

No description provided.

@merks
Copy link
Contributor Author

merks commented May 6, 2022

@akurtakov How does one make sense of (determine the cause of) the License check failure?

@merks
Copy link
Contributor Author

merks commented May 7, 2022

I see, it's complaining here so

https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/runs/6321822995?check_suite_focus=true#step:4:24854

 [INFO] p2/orbit/p2.eclipse.plugin/javax.xml/1.4.1.v20220503-2331

So this is unrelated to this PR.

@merks
Copy link
Contributor Author

merks commented May 9, 2022

@akurtakov Is this okay?

@akurtakov
Copy link
Member

I don't like the idea. And I said that in the mailing list already (https://www.eclipse.org/lists/cross-project-issues-dev/msg19144.html) - JDT/PDE are usually one of the very first adopters of asm so what happens next time? Force us to contribute to Orbit or we go back to using Maven artifact directly and when someone puts in Orbit you do the dance back to use Orbit?

@merks
Copy link
Contributor Author

merks commented May 9, 2022

I understand the concern.

Yes, next time you can do exactly the same thing as this time, i.e., use the Maven version directly. And, next time, if someone (like me) bothers to help and do the work of ensuring there is an Orbit version and also does the work to use it in the platform's *.target file then that someone can do the work again, or not. I.e., there is no future obligation, on anyone's part. For this time, I've already done the work....

Okay?

@merks
Copy link
Contributor Author

merks commented May 9, 2022

Maybe I change this commit to only comment out the other part so that in the future it's really easy to use the Maven artifacts again/instead? I definitely don't want to make anything more difficult for anyone!

@akurtakov
Copy link
Member

Fine with me from releng POV. I would like to hear that @vik-chand, @jarthana or @iloveeclipse don't have concerns with such approach as the bundles themself are used in PDE/JDT respectively.
Yes, It would be much appreciated to only comment the maven part.

@iloveeclipse
Copy link
Member

I personally have no concerns using orbit versions. I see ASM is used by Eclemma / Xtext and there is a good reason to keep on same version for all.

@cdietrich
Copy link

what needs to be done to move the "maven thing" to orbit?

@merks
Copy link
Contributor Author

merks commented May 9, 2022

what needs to be done to move the "maven thing" to orbit?

Nothing needs to be done. All these are already in the latest Orbit I-build:

image

The platform simply needs to use them.

@akurtakov

This latest PR has commented sections for the two alternative ways of including the parts of ASM needed by the platform (JDT and PDE) in the target platform such that its easy to switch to use either form in the future.

@cdietrich
Copy link

@merks i mean if the maven thing is 1000% easier than the adding stuff to orbit with ebr. maybe we should leverage the "maven thing" on orbit side

@akurtakov
Copy link
Member

what needs to be done to move the "maven thing" to orbit?

Even if ^^ are fixed that it would be 3 times (patch for Orbit, build Orbit, update Platform target) more work compared to "update Platform target" directly. This 3 times is just the direct time spent to prepare patches but if we count the fact that one has to wait for several more builds, builds published, account for other changes in Orbit, touch features for that and etc. - it is 5 times more work in the very best case. Not a thrilling alternative!

@akurtakov
Copy link
Member

This latest PR has commented sections for the two alternative ways of including the parts of ASM needed by the platform (JDT and PDE) in the target platform such that its easy to switch to use either form in the future.

@merks If there is no objection by JDT/PDE leads by this time tomorrow please merge.

@merks
Copy link
Contributor Author

merks commented May 9, 2022

@merks i mean if the maven thing is 1000% easier than the adding stuff to orbit with ebr. maybe we should leverage the "maven thing" on orbit side

@cdietrich I don't understand why you are asking this. Did we not already exchange direct emails about how to consume directly from Maven

I really don't want to have discussions about something being 1000% easier because it's a question of easier for whom and then a question how many people must replicate the easier thing and then how many people/times that needs to be replicated before overall the question of easier versus harder becomes moot.

I just want to make sure that things are easy/easier for @akurtakov and the platform team first and foremost, and then also easier for everyone else if that's possible in combination. So what's done in this PR seems a good compromise...

@cdietrich
Copy link

so we will have the same problem with 9.4 again

@merks
Copy link
Contributor Author

merks commented May 9, 2022

Coordinating versions is always a problem. I am trying to help make it less of a problem as best I can. Is there something else/more I should do?

@cdietrich
Copy link

to me this is about the broader direction in which simrel is going. if platform decides to abolish orbit cause not using orbit is easier and others cant follow we will be in the same position over and over again

@akurtakov
Copy link
Member

to me this is about the broader direction in which simrel is going. if platform decides to abolish orbit cause not using orbit is easier and others cant follow we will be in the same position over and over again

I appreciate the discussion but it simply doesn't belong in this PR. This is what cross-project mailing list is for.

@cdietrich
Copy link

cdietrich commented May 9, 2022

mailing lists really suck for such discussions
should i start a new one there or pick one of the old ones? or simply rage quit or ignore it and dont care anymore

@akurtakov
Copy link
Member

mailing lists really suck for such discussions should i start a new one there or pick one of the old ones? or simply rage quit or ignore it and dont care anymore

My very last comment on the topic here.
If the simrel itself doesn't have a good way for coordination and discussions - maybe this is the very first thing that should be discussed.
Regarding the Orbit future or whether it has any - if it was me I would start new conversation as very long threads make very few people read them and eventually no one .
Right now the only channel I know for these discussions is the cross-project mailing list.

@HannesWell
Copy link
Member

mailing lists really suck for such discussions should i start a new one there or pick one of the old ones? or simply rage quit or ignore it and dont care anymore

We should enable GH-discussions for this repo as well (or actually for all Eclipse repos), IMHO this is a better place to discuss something like this compared to this PR and the Mailing-List.

@akurtakov
Copy link
Member

mailing lists really suck for such discussions should i start a new one there or pick one of the old ones? or simply rage quit or ignore it and dont care anymore

We should enable GH-discussions for this repo as well (or actually for all Eclipse repos), IMHO this is a better place to discuss something like this compared to this PR and the Mailing-List.

I totally disagree that future of Simrel collaboration should be described at Eclipse Platform channel! These channels are for development of Eclipse Platform itself.

@merks merks merged commit e1f583c into eclipse-platform:master May 10, 2022
@merks merks deleted the pr-orbit-asm branch May 10, 2022 07:48
@jonahgraham
Copy link
Contributor

@merks i mean if the maven thing is 1000% easier than the adding stuff to orbit with ebr. maybe we should leverage the "maven thing" on orbit side

@cdietrich I don't understand why you are asking this. Did we not already exchange direct emails about how to consume directly from Maven

@cdietrich / @merks - this has been proposed by @vogella in Orbit bugzilla, see Bug 568936 Comment 27 and 28

@vogella
Copy link
Contributor

vogella commented May 12, 2022

@cdietrich / @merks - this has been proposed by @vogella in Orbit bugzilla, see Bug 568936 Comment 27 and 28

@jonahgraham technical this change should be trivial:

  • Define Target platform with the Maven libs as depencies
  • Add desire Feature name to Target platform
  • Create category.xml in another project including this feature
  • Create / adjust Tycho based build

Is such a change desired for Orbit? If yes, please open an issue / bug, cc me and we can start working on this.

@jonahgraham
Copy link
Contributor

Is such a change desired for Orbit? If yes, please open an issue / bug, cc me and we can start working on this.

Possibly (hopefully?) - lets continue this in this new bug: Bug 579916

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

7 participants