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

Merge repo into eclipse.platform.ui #31

Closed
vogella opened this issue Feb 10, 2023 · 61 comments
Closed

Merge repo into eclipse.platform.ui #31

vogella opened this issue Feb 10, 2023 · 61 comments

Comments

@vogella
Copy link
Contributor

vogella commented Feb 10, 2023

I suggest to merge this one into eclipse.platform.ui. Any objections? cc @laeubi and @akurtakov

@akurtakov
Copy link
Member

Every single simplification matters and there are way too many obstacles for many to land so if you can do this one - go for it.

@laeubi
Copy link
Contributor

laeubi commented Feb 10, 2023

Sounds reasonable.

@vogella
Copy link
Contributor Author

vogella commented Feb 14, 2023

Maybe better target directly eclipse.platform repo? This seems our all-in-one final target anyway. IMHO the best (releng) option is to end up with one repo.

@akurtakov
Copy link
Member

IMHO put it in UI so the two merges can go simultaneosly now :) . We are far from targetting one repo yet so let's aim for best shortterm gain.

@laeubi
Copy link
Contributor

laeubi commented Feb 14, 2023

Last time it was said that we should aim for

  • platform
  • platform.ui

as a first step because of build times and separation.

@akurtakov
Copy link
Member

Last time it was said that we should aim for

* platform

* platform.ui

as a first step because of build times and separation.

I stand behind this 100 % for now.

@vogella
Copy link
Contributor Author

vogella commented Feb 14, 2023

So the merged repo goes into a "tools" subdirectory in platform.ui as we do for merges in platform?

@akurtakov
Copy link
Member

Sounds good to me.

@merks
Copy link
Contributor

merks commented Feb 14, 2023

I'm not sure all these things are tools? The source repo was "ua" for user assistance. It seems to me all these things are mostly framework components, not tools...

@laeubi
Copy link
Contributor

laeubi commented Feb 14, 2023

@merks I think if we found that the previous categories are wrong, we can easily move the items around to a more suitable place after the merge.

Alternative would be to help @vogella to move them to a proper place as part of the merge (but I can't decide on that as I don't know that in detail)

@akurtakov
Copy link
Member

akurtakov commented Feb 14, 2023

Someone named them tools (even bundle names are o.e.e4.tools.*) so even if being general components tools is still good place in the sense of folder with o.e.e4.tools.* components.
If the question is about renaming the bundles and etc. that's far bigger discussion which shouldn't interfere with merging repos.

@merks
Copy link
Contributor

merks commented Feb 14, 2023

I don't have a really strong opinion but at some point "ua" seemed like good name (to some human beings) for some group of things, and perhaps folks looking for things in that group might not be helped by looking in tools. We are talking about these, right?

image

No sign of tools here. Just saying. 😉

@vogella
Copy link
Contributor Author

vogella commented Feb 14, 2023

@merks you commenting on the wrong issue, this is the merge from ui.tools not eclipse-platform/eclipse.platform#324 which you are referring too.

@merks
Copy link
Contributor

merks commented Feb 14, 2023

Sorry !!! Now it all makes perfect sense. Sorry sorry.

@akurtakov
Copy link
Member

Happens to me quite often thanks to multitasking :)

@vogella
Copy link
Contributor Author

vogella commented Feb 14, 2023

Lets see what CI error we get this time eclipse-platform/eclipse.platform.ui#602

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

Done

@vogella vogella closed this as completed Feb 20, 2023
@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

FYI, AFAICS history for the merged files is still available.

image

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

For existing files from platform.ui

image

@merks
Copy link
Contributor

merks commented Feb 20, 2023

Why were maven natures and filters added to the .project files?

image

Also, why do we need .polyglot.META-INF?

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

@merks the maven natures were added automatically by the pde-m2e tooling. Fighting such automation is time consuming. cc @laeubi

I will remove the .poly... stuff and add the file to the .gitignore.

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

@merks where do you see .polyglot.META-INF? find . -name ".polyglot.META-INF" does not find anything on my machine.

@merks
Copy link
Contributor

merks commented Feb 20, 2023

Probably/maybe m2e is generating them?

@akurtakov
Copy link
Member

These are runtime artifacts of tycho when using pomless builds and they are deleted at end of build. As visible from the screenshots they are ignored by git.

@merks
Copy link
Contributor

merks commented Feb 20, 2023

I've never done a Tycho build so I don't know where they came from. An even bigger problem is that all the .project files are missing for the tools projects:

image

@merks merks reopened this Feb 20, 2023
@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

@merks Thanks for noticing. Fixed via eclipse-platform/eclipse.platform.ui#605

@laeubi
Copy link
Contributor

laeubi commented Feb 20, 2023

When the maven nature is enabled, the files are created as part of the project import, so I think the issue is more that the .project files are missing, then they are handled as maven projects and then you get the maven nature and the temp files (of course you can ignore them).

I once suggested that git ignored files can be automatically filtered from the view, but I think this was never considered further.

@merks
Copy link
Contributor

merks commented Feb 20, 2023

Now I see problems with "dirty" state after these import successfully.

image

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

In case no one has idea how to deal with JDT dependency, simplest solution would be to disable building org.eclipse.e4.tools.jdt.templates & remove if from org.eclipse.e4.core.tools.feature.

Yeah, I'm doing this right now....

eclipse-platform/eclipse.platform.ui#608

@akurtakov
Copy link
Member

akurtakov commented Feb 20, 2023

So if one does just mvn clean verify (muscle memory) build works, one has to explictly pass -DaggregatorBuild=true to see the issue. This explains why issue was not catched.

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

So if one does just mvn clean verify (muscle memory) build works, one has to explictly pass -DaggregatorBuild=true to see the issue. This explains why issue was not catched.

I also do not see a GH code verification action for PR in the aggregator repo https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/tree/master/.github/workflows

@laeubi
Copy link
Contributor

laeubi commented Feb 20, 2023

I'll try to take a look at the build issue as soon as I'm finished with some tasks, but probably one want to enable that option for that repo?

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

JDT templates removed from the build and re-triggered I-build https://ci.eclipse.org/releng/job/Builds/job/I-build-4.27/140/

@iloveeclipse
Copy link
Member

JDT templates removed from the build and re-triggered I-build

@vogella: If that "JDT templates" thing is supposed to be useful for e4, please create an issue to enable them again.

@iloveeclipse
Copy link
Member

Now we face xpath issue again:

14:29:39  [ERROR]   Missing requirement: org.eclipse.e4.ui.model.workbench 2.3.0.qualifier requires 'osgi.bundle; org.eclipse.e4.emf.xpath 0.1.100' but it could not be found

@merks
Copy link
Contributor

merks commented Feb 20, 2023

I think that project got deleted maybe?

I'm having trouble finding it and dependencies on it in my workspace resolve to the binary plugins in the target platform not to a workspace project.

image

@iloveeclipse
Copy link
Member

I think that project got deleted maybe?

That happened at very beginning with 708cf228e289b3dd96416ccc6b153c9606cec620
@vogella : why?

@akurtakov
Copy link
Member

No jxpath no CVEs :).

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

No jxpath no CVEs :).

I restorted it with eclipse-platform/eclipse.platform.ui#610, this was unintentially deleted and as the features live in another repo then the plug-ins I did not notice.

@akurtakov
Copy link
Member

Now seriously, this shows how big of an issue our many repositories with many cyclic references repositories is if such an issue happens and the subrepo build tells us all is fine.

@akurtakov
Copy link
Member

I restorted it with eclipse-platform/eclipse.platform.ui#610, this was unintentially deleted and as the features live in another repo then the plug-ins I did not notice.

I think you should revert the jdt.templates change too.

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

I restorted it with eclipse-platform/eclipse.platform.ui#610, this was unintentially deleted and as the features live in another repo then the plug-ins I did not notice.

I think you should revert the jdt.templates change too.

Lets do this after the other issue is solved.

@merks
Copy link
Contributor

merks commented Feb 20, 2023

Odd that the test dependency on it didn't fail the build...

image

@akurtakov
Copy link
Member

It was satisfied just fine from I-builds repo where the bundle exists so no reason for the test bundle to not resolve.

@merks
Copy link
Contributor

merks commented Feb 20, 2023

@akurtakov Ah, now it all make sense! Thanks.

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

eclipse-platform/eclipse.platform.ui#610 merged, next try. Unfortunately I be away from the laptop soon, so if any work is still requred I can look at it tomorrow.

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

Would be more effective if the source bundle creation could be after the real compilation.

image

@akurtakov
Copy link
Member

After the build repos are not clean so either it would have to be cleaned or even worse repos would have to be cloned again. This raise the question who needs the source tarball anymore? It was added by Red Hat and used for the Fedora/RHEL rpm builds which we no longer use so maybe no one uses them at all.

@vogella
Copy link
Contributor Author

vogella commented Feb 20, 2023

@akurtakov I created eclipse-platform/eclipse.platform.releng.aggregator#882 to retire the source tarball generation.

@vogella
Copy link
Contributor Author

vogella commented Feb 21, 2023

Build successful and AFAICS no new test errors ( https://download.eclipse.org/eclipse/downloads/drops4/I20230219-1800/testResults.php) , so lets close this. Please open new issues in in eclipse.platform.ui if you find another issue.

Thanks for the help to the involved people.

@vogella vogella closed this as completed Feb 21, 2023
@akurtakov
Copy link
Member

Would you please revert the jdt.templates removal from feature?

@vogella
Copy link
Contributor Author

vogella commented Feb 21, 2023

Would you please revert the jdt.templates removal from feature?

eclipse-platform/eclipse.platform.ui#612

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

No branches or pull requests

5 participants