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

Eclipse can't hack Elasticsearch #53664

Closed
4 of 7 tasks
nik9000 opened this issue Mar 17, 2020 · 10 comments
Closed
4 of 7 tasks

Eclipse can't hack Elasticsearch #53664

nik9000 opened this issue Mar 17, 2020 · 10 comments
Labels
:Delivery/Build Build or test infrastructure Meta Team:Delivery Meta label for Delivery team

Comments

@nik9000
Copy link
Member

nik9000 commented Mar 17, 2020

Using Eclipse to hack on Elasticsearch is broken. Its been broken to varying degrees for a long time, but it is slowly, slowly, slowly, getting more broken. A bunch of us have been hacking Elasticsearch with Eclipse for a long time and will continue to do so. But we can't continue to recommend Eclipse to new contributors. The team would love to support Eclipse because we feel like that is the right thing to do. And because some of us are used to Eclipse and would like to continue hacking with it. So this meta issue will track what we have to do to get Eclipse working again. To the point where we can recommend it to new contributors again.

The trouble is that we don't expect we'll have time to really fix most of the Eclipse issues soon. This'll stay open for a long time. And that entire time Eclipse will be officially "not recommended".

TODO:

  • Drop Eclipse support from CONTRIBUTING.md (@nik9000)
  • Fix issues with "Import Gradle Project" so it works as well as `./gradlew eclipse && Import Eclipse Project"
  • Remove ./gradlew eclipse. It will never support test dependencies (that is a buildship feature. It should be supported by "Import Gradle Project". We think.
  • Implement test dependencies. This should solve the jarhell issues we see running tests in Eclipse. Most of them, at least. (Add new instruction for Eclipse #54894)
  • Have some kind of automatic test to make sure that we don't break Eclipse in the future. Maybe it'd just use the language server to import the projects and run some tests or something. Dunno.
  • Other stuff?
  • Add Eclipse support to CONTRIBUTING.md (Add new instruction for Eclipse #54894)
@nik9000 nik9000 added :Delivery/Build Build or test infrastructure Meta labels Mar 17, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Build)

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Mar 17, 2020
Hacking Elasticsearch with Eclipse is fairly broken. Some things work,
but we can't in good concience point contributors to Eclipse. This drops
the instrucions for Eclipse from CONTRIBUTING.md. We *do* want to fix
Eclipse. But we don't see that happening in the next few months. Once we
*have* fixed Eclipse we'll rewrite the instructions.

Relates elastic#53664
nik9000 added a commit that referenced this issue Mar 17, 2020
Hacking Elasticsearch with Eclipse is fairly broken. Some things work,
but we can't in good concience point contributors to Eclipse. This drops
the instrucions for Eclipse from CONTRIBUTING.md. We *do* want to fix
Eclipse. But we don't see that happening in the next few months. Once we
*have* fixed Eclipse we'll rewrite the instructions.

Relates #53664
@costin
Copy link
Member

costin commented Mar 19, 2020

Remove ./gradlew eclipse. It will never support test dependencies

Some test dependencies are marked as visible for testing only - see the screenshot below (the jars have a darker color).

image

@nik9000
Copy link
Member Author

nik9000 commented Mar 19, 2020 via email

@nik9000
Copy link
Member Author

nik9000 commented Mar 19, 2020

Remove ./gradlew eclipse. It will never support test dependencies (that is a buildship feature. It should be supported by "Import Gradle Project". We think.

One of the problems with buildship is that it doesn't have any way to configure settings for things like "circular dependencies are warnings not errors".

@nik9000
Copy link
Member Author

nik9000 commented Mar 19, 2020

Another interesting problem with "Import Gradle Project" - it doesn't configure stuff like "always use spaces instead of tabs".

@rjernst rjernst added the Team:Core/Infra Meta label for core/infra team label May 4, 2020
@mark-vieira mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020
@mark-vieira
Copy link
Contributor

@nik9000 where did we land on this. From what I recall you were able to resolve most if not all of the issues here.

@nik9000
Copy link
Member Author

nik9000 commented Dec 18, 2020 via email

@mark-vieira
Copy link
Contributor

It costs us a couple hours a week but that is about the best we can do.

I find this surprising. Are you saying that you lose a couple hours every week just due to Eclipse issues? I mean, we certainly don't spend anywhere near that much time dealing with IntelliJ issues. Or is it just the fact that random stuff incidentally breaks Eclipse (but not IntelliJ) that requires you to invest time in fixing it?

@nik9000
Copy link
Member Author

nik9000 commented Jan 29, 2021

I think I wrote that in anger after having spent a couple days on Eclipse stuff. I think a better estimate would be several hours a month. Stuff certainly does go wrong from time to time and its annoyingly unpredictable. You pull master and, bam, you can't compile lots of stuff. This is mostly Elipse's fault. And, mostly its around dependencies and source roots. Or maybe it's buildship's fault for trying to have a one to one relationship between Eclipse project and gradle project - an Eclipse project is much more like a single source root than a gradle project.

I do have a sneaking suspicion the IntelliJ has all kinds of different weird stuff but dealing with that usually falls on the person who is making the change to ES because they see IntelliJ not working and do something about it. But Eclipse maintenance falls to a small group of folks who use Eclipse.

ide.gradle configures a lot of things for IntelliJ and very little for Eclipse. Mostly, again, this is Buildship's fault for not letting us configure that stuff.

Whatever the cause I'd love to do something better but I'm ok with where we are. Without warning I sometimes have to stop what I'm doing and spend anywhere from half an hour to two days on fixing up Eclipse stuff. The short incidents happen about once a month. The long ones more rarely. Twice a year or so.

We have at times reverted things because the blew up Eclipse just to give us Eclipse folks some breathing room. But we don't tend to because Eclipse folk are used to fixing it.

@mark-vieira
Copy link
Contributor

Ok, I realize the status quo is not great but the reality is, testing every change that might possibly impact IDE integration on both IntelliJ and Eclipse is not something we are going to start doing. We rely on the Eclipse users to surface these issues, at the cost of some pain on your end. The basic thing is our build is "optimized" for Intellij, and honestly the number of changes the break IntelliJ are very few and far between because the Gradle/IntelliJ models are so much more similar.

I wish I had a better answer here, or that we had more resources to make Eclipse a first class citizen here but I won't pretend that's the case. If you're fine with it I'm going to close this for now. Of course, as issues arise, feel free to reach out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Meta Team:Delivery Meta label for Delivery team
Projects
None yet
Development

No branches or pull requests

5 participants