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

Line ending confusion in generated files of Xtext project? #2778

Closed
Bananeweizen opened this issue Aug 11, 2023 · 50 comments · Fixed by #3005
Closed

Line ending confusion in generated files of Xtext project? #2778

Bananeweizen opened this issue Aug 11, 2023 · 50 comments · Fixed by #3005

Comments

@Bananeweizen
Copy link
Contributor

When I create a new Oomph setup for the Xtext project on my Windows machine, then Oomph automatically sets autocrlf=true, even though there is no such attribute in the *.setup file:
grafik

That in turn leads to all the Xtend generated Java files having CRLF in my local index (while the Github repo uses LF). But during build and re-generation all these files get LF only (which fits to the projects having the Unix line ending setting in their project properties). Therefore the IDE has 800+ changed files after a restart, almost all of them with newline changes only, looking like this:
grafik

I do use a personal Oomph setup, but I'm not aware of any setting that would cause Oomph to add the autocrlf just for me, so I suspect this should be broken similarly for everyone on Windows. If that is true, would it be useful to explicitly set autocrlf to another value in the *.setup or alternatively to set *.java text eol=lf in a .gitattributes file eventually? I've used the gitattributes approach locally, reset hard, and since then my staging area has only 24 changes files, where most contain actual non whitespaces changes (which would be another thing to investigate later).

@cdietrich
Copy link
Contributor

cdietrich commented Aug 12, 2023

this issue is new to me.
i am only aware on windows changes on fresh workspaces caused by
#2293
back when i still had a windows pc this was the only changes i had.
is this oomph setting more recently introduced?

unfortunately with company change i have no suiteable windows machine anymore to have a look.

@Bananeweizen
Copy link
Contributor Author

The Windows changes that you linked are in the above mentioned remaining 24 changed files after I applied the gitattributes. I can try to have a colleague set up another Xtext installation on his machine next week to verify he sees the same.
My initial thought was that this may have been caused by the monorepo change some weeks/months ago, but I have not been able to pinpoint to any changes in the .setup or in the archived repos that would support that claim.

@cdietrich
Copy link
Contributor

maybe a default on oomph or egit side has changed?

@pbrossel
Copy link

Hi @Bananeweizen,
can you tell me what changes you have made to temp fix the issue ?
I did an initial oomph setup on windows and have exactly the same problem as you had.

...
I've used the gitattributes approach locally, reset hard, and since then my staging area has only 24 changes files, where most contain actual non whitespaces changes (which would be another thing to investigate later).

@LorenzoBettini
Copy link
Contributor

I've just tried the Oomph setup in Windows as well. I mostly use Linux and sometimes macOS, while I rarely used Windows.

I'm experiencing the same problem as well!

We are aware of a few Xtend generated files with Windows EOLs, which is #2293 for which I recently proposed a fix #3004

However, I experienced the bigger problem reported here, I seem to understand:

  • after the first Oomph run, there are only a few generated Xtend files that are dirty (because of the generated "\r" in the Java code's strings)
  • as soon as I restart the Eclipse workspace, Oomph insists in always re-running our task "Projects Build (Refresh, Clean, Build)" (@merks any idea why? This does not happen in Linux and macOS; why does Oomph always re-execute that task?) which then leads to tons of dirty files, not related to Wrong line delimiter when enclosing multi-line strings with double quotes. #2293 but to the problem initially reported in this bug.

I seem to understand that "autocrlf=true" is the correct setting in Windows, and @HannesWell also recently provided a merged PR normalizing a few files with incorrect line endings.

Of course, I can reset hard, but this is very annoying.

@Bananeweizen says that

But during build and re-generation all these files get LF only (which fits to the projects having the Unix line ending setting in their project properties).

So maybe we should not have that setting in our projects?

@LorenzoBettini
Copy link
Contributor

A few more experiments (in Windows): the projects where this happens (again, independently from #2293) seem to be where we have /org.eclipse.xtend.ide/.settings/org.eclipse.core.runtime.prefs with these contents>

eclipse.preferences.version=1
line.separator=\n

Removing the file and rebuilding fixes the problem in that project.

@cdietrich I can provide a PR removing all such files, what do you think?

@cdietrich
Copy link
Contributor

unfortunately i have no idea.
i assume m2e or something else will replace this again

@LorenzoBettini
Copy link
Contributor

I see those files were added here https://github.com/search?q=repo%3Aeclipse%2Fxtext+Set+UNIX-style+linebreaks&type=commits but I see no reason why they are there.

No, @cdietrich , the files you're mentioning, which are correctly regenerated/updated, are /org.eclipse.xtend.ide/.settings/org.eclipse.core.resources.prefs; the ones I talk about, which are not even everywhere, are /org.eclipse.xtend.ide/.settings/org.eclipse.core.runtime.prefs (runtime vs resources).

@cdietrich
Copy link
Contributor

@szarnekow do you remember why its this way

@LorenzoBettini
Copy link
Contributor

With this #3005 on my Windows system, even a clean build does not generate all the dirty files (only seven remain due to #2293)

@HannesWell
Copy link
Contributor

HannesWell commented Apr 22, 2024

Probably having autocrlf=truein the git config is overall the better way to keep the line endings consistent in git. Therefore I think your proposal is a good one.

the files you're mentioning, which are correctly regenerated/updated, are /org.eclipse.xtend.ide/.settings/org.eclipse.core.resources.prefs; the ones I talk about, which are not even everywhere, are /org.eclipse.xtend.ide/.settings/org.eclipse.core.runtime.prefs (runtime vs resources)

The resource preferences are even generated by the eclipse core (probably even the resources plugin itself). This changed a few releases ago.

Regarding m2e updating the configuration: Automatic updates can be disabled in the workspace preferences and since the last release even in the project preferences.

@LorenzoBettini
Copy link
Contributor

@HannesWell I'm talking about "runtime.prefs", not "resources.prefs".

@HannesWell
Copy link
Contributor

Yes. My second paragraph was just for information in case you didn't knew it already and it is of interest for you.

@szarnekow
Copy link
Contributor

@szarnekow do you remember why its this way

I can imagine that it was done to get a stable compilation result on windows and linux machines. This is only possible if files are checked-out as-is and line-breaks are always generated with the same EoL sequence (unix). If we use autocrlf a compilation on windows would yield different results than the compilation on unix. That's not desirable.

@szarnekow
Copy link
Contributor

szarnekow commented Apr 23, 2024

different results

Meaning if trace-files are different since the offsets would be different on windows and unix

@LorenzoBettini
Copy link
Contributor

As far as I know, not using autocrlf=true on Windows always leads to troubles; indeed, a few files ended up with the wrong EOL in the past (fixed by @HannesWell).

By removing the settings of Unix EOLs nothing broke.

@merks
Copy link
Contributor

merks commented Apr 23, 2024

Start rant.

Here's what my experience says. Tell everyone on Windows that they should use only Linux line endings. Then if the tools are bad and generate only Linux line endings rather than respecting your settings, you won't notice, the tools will just remain broken, and the tools will never be fixed. But that makes everyone happy, especially the people not on windows, so just do it. 😱

End rant.

Note though that minimally you should use autocrlf=input because tools can be bad in multiple ways. Some tools are bad that on Windows they always produce windows line endings because that's what the system property tells them to do. As far as I can tell it's impossible to set the system property to a different value via a command line argument. After all, who would ever want to do that, other than me? So if you're not careful, your solution to the problem is exactly what will end up committing windows line endings into the repository causing your solution to be the new problem.

I can see many bad solutions in the project catalog:

image
image

Here is a good one:

image

@LorenzoBettini
Copy link
Contributor

@merks I don't understand what you're suggesting.
Use autocrlf=input also in Windows? As I said, Git itself suggests to do autocrlf=true on Windows. Or am I missing something?
The dirty files on Windows were due to the additional settings we have for forcing Unix EOLs, which should be useless nowadays, that's why I proposed #3005

@merks
Copy link
Contributor

merks commented Apr 23, 2024

I'd leave it as true. I wasn't sure where this thread was going, didn't have the time to read it in detail, but was hoping it would not lead to simply removing the setting because you can see how many other projects have reached that questionable conclusion. Sorry for the noise.

@LorenzoBettini
Copy link
Contributor

@merks no problem, and thanks for the confirmation :)
I wonder whether the problems I mentioned above about Oomph always running the clean build step in Windows might be due to the Unix EOLs Eclipse project settings we have. I'll try again later when I have the Windows machine.

@merks
Copy link
Contributor

merks commented Apr 23, 2024

I think you've specified the task such that it always kicks in when you start.

image

Maybe you want it only for new projects added to the workspace?

@LorenzoBettini
Copy link
Contributor

Mh.. that might be the cause, true! But the problem happens only in Windows, not in Linux, nor macOS...

@merks
Copy link
Contributor

merks commented Apr 23, 2024

"The problem" being it always runs?

@LorenzoBettini
Copy link
Contributor

Yes, exactly: when you restart Eclipse, it will always run; but only on Windows.

@merks
Copy link
Contributor

merks commented Apr 23, 2024

I see. That's quite unexpected! I hate when OSes behave differently. It's too much work to track down such problems...

@LorenzoBettini
Copy link
Contributor

I'll check whether the Unix EOL settings were the culprit, when I'm on a Windows machine

LorenzoBettini added a commit that referenced this issue Apr 29, 2024
This should deal with the problem of always getting a clean build in Windows, each time you open the development workspace.

See #2778 (comment)
@LorenzoBettini
Copy link
Contributor

@Bananeweizen @pbrossel everything should be fixed now (together with #3004)

Oomphing from scratch or updating with Oomph after updating to main should give you a clean workspace without dirty files on Windows.
IMPORTANT: on Windows you must have autocrlf=true

@szarnekow
Copy link
Contributor

IMPORTANT: on Windows you must have autocrlf=true

Is this already part of the oomph setup?

@LorenzoBettini
Copy link
Contributor

IMPORTANT: on Windows you must have autocrlf=true

Is this already part of the oomph setup?

@szarnekow I don't think we can enforce that on a per-OS basis. Can we @merks ?

@merks
Copy link
Contributor

merks commented Apr 30, 2024

Oomph's git clone task does this configuration by default for every clone unless you have an explicit configuration for that in the git clone task itself. With some trickery I think it's possible do set it differently only different OSes, but given it's a no-op on Linux and Mac and I believe even there it would prevent checking in Windows line endings, so I don't see a need to configure it differently on different OSes.

@LorenzoBettini
Copy link
Contributor

@merks so, if I understand correctly, by default Oomph sets autocrlf=true in Windows and does nothing in macOS and Linux, is that correct? If so, we're all fine.

@pbrossel
Copy link

pbrossel commented May 3, 2024

@Bananeweizen @pbrossel everything should be fixed now (together with #3004)

Oomphing from scratch or updating with Oomph after updating to main should give you a clean workspace without dirty files on Windows. IMPORTANT: on Windows you must have autocrlf=true

Thanks a lot to all having worked on this issue !
Setup tasks are now finished quickly without rebuilding everything :
image

But there are some questions / remarks :

  • I moved to my fork by replacing the contents of xtext-main\git\xtext : Is this the right approach ?
  • I had to add autocrlf = true manually, maybe because I cloned the fork manually ?
  • still had tons of fake changes even after resetting to main. Many restarts or clean/rebuild did not help.
  • target platform had errors. Editing and reapplying the definition helped to fix compile errors, but I got a warning that I should use a newer Eclipse version (currently using Version: 2024-03 (4.31.0) Build id: 20240307-1437 )
  • After reset hard + clean rebuild, I finally had 17 'fake' changes left over. Is that OK ?
org.eclipse.xtend.core.tests/xtend-gen/org/eclipse/xtend/core/tests/EmfModelsTest.java
org.eclipse.xtend.core/xtend-gen/org/eclipse/xtend/core/macro/ConstantExpressionsInterpreter.java
org.eclipse.xtend.core/xtend-gen/org/eclipse/xtend/core/macro/TransformationContextImpl.java
org.eclipse.xtend.core/xtend-gen/org/eclipse/xtend/core/macro/ValidationContextImpl.java
org.eclipse.xtend.core/xtend-gen/org/eclipse/xtend/core/macro/declaration/AnnotationReferenceBuildContextImpl.java
org.eclipse.xtend.core/xtend-gen/org/eclipse/xtend/core/macro/declaration/CompilationUnitImpl.java
org.eclipse.xtend.examples/projects/xtend-euler/xtend-gen/euler/Solution_004.java
org.eclipse.xtend.examples/projects/xtend-euler/xtend-gen/euler/Solution_014.java
org.eclipse.xtend.examples/projects/xtend-euler/xtend-gen/euler/Solution_017.java
org.eclipse.xtend.examples/projects/xtend-euler/xtend-gen/euler/Solution_021.java
org.eclipse.xtend.examples/projects/xtend-examples/xtend-gen/example2/BasicExpressions.java
org.eclipse.xtend.ide.swtbot.tests/swtbot/xtend-gen/org/eclipse/xtend/ide/tests/SwtBotProjectHelper.java
org.eclipse.xtend.ide/xtend-gen/org/eclipse/xtend/ide/codebuilder/AbstractCodeBuilder.java
org.eclipse.xtend.lib/xtend-gen/org/eclipse/xtend/lib/annotations/DelegateProcessor.java
org.eclipse.xtend.lib/xtend-gen/org/eclipse/xtend/lib/annotations/EqualsHashCodeProcessor.java
org.eclipse.xtext.xbase/xtend-gen/org/eclipse/xtext/xbase/compiler/JvmModelGenerator.java
org.eclipse.xtext.xbase/xtend-gen/org/eclipse/xtext/xbase/controlflow/ConstantConditionsInterpreter.java
  • is there a documentation somewhere about the steps to achieve to catch up with a new change or milestone ?
  • when starting the ide, there is still something building, even if no changes occurred. Takes some time but I can live with it. Is that OK ?

@LorenzoBettini
Copy link
Contributor

@pbrossel Very quick answer about the crucial part (I'll try to answer the other questions ASAP): if you clone a repository manually, core.autocrlf=true must be set appropriately in the user global git configuration (~/.gitconfig), and, IIRC, it must be done before cloning. Concerning the fake changes, are they really fake or do they still contain differences? Note that you must have the latest Xtend in your IDE so that you have the version with these changes #3004.

@pbrossel
Copy link

pbrossel commented May 3, 2024

@LorenzoBettini

Very quick answer about the crucial part (I'll try to answer the other questions ASAP): if you clone a repository manually, core.autocrlf=true must be set appropriately in the user global git configuration (~/.gitconfig), and, IIRC, it must be done before cloning.

I'll retry it according to your advice.

Concerning the fake changes, are they really fake or do they still contain differences?

I thought they were 'fake' changes, but it could be the result of using the wrong version of xtend in the ide.

image

At least these 4 generated sources come back to the original version after installing the IDE updates.

image

Note that you must have the latest Xtend in your IDE so that you have the version with these changes #3004.

Where can I check if the changes are contained in the updates ?
image

@LorenzoBettini
Copy link
Contributor

@pbrossel your version of Xtend is the latest one, which contains #2993, where @HannesWell replaced the use of Guava Object.equal with Java Object.equals.

After you install the new version of the Xtend compiler, as you have already done, you have to trigger regeneration in the projects showing dirty files (since now the full build clean Oomph task is not triggered anymore on existing projects, as you saw above). There's no need to fully clean the whole workspace: only cleaning the single projects with dirty Java files is enough.

Please, let me know.

@pbrossel
Copy link

pbrossel commented May 4, 2024

@LorenzoBettini

After you install the new version of the Xtend compiler, as you have already done, you have to trigger regeneration in the projects showing dirty files (since now the full build clean Oomph task is not triggered anymore on existing projects, as you saw above). There's no need to fully clean the whole workspace: only cleaning the single projects with dirty Java files is enough.

Please, let me know.

I first removed the 'fake' changes, and rebased on your latest commit and got no changes at all : ok.

image

I then cleaned and rebuilt the first project in the list of fake changes org.eclipse.xtend.core and got changes in these 8 files :

image

Opening the compare editor by double clicking on the first unstaged change shows no difference on the java file.

image

Opening the working tree version using the context menu on the first unstaged change opens the xtend source.
Comparing with the latest commit shows '1 difference' being something like a fake (i.e. the button next difference does not show anything).

image

The button next change shows indeed that something is wrong :

image

So I guess that there are still some fake changes around.

I get 219 changes with the same symptoms after having done a full clean rebuild :

image

Note that I did not clone again after setting user global git configuration to autocrlf=true

I'll try to start from scratch with a fresh clone and see if it solves the problem.

@pbrossel
Copy link

pbrossel commented May 4, 2024

@LorenzoBettini

Even if setting the core.autocrlf=true in the user settings before cloning (via Egit), the clone's config does not contain the setting.

image

I now added the core.autocrlf=true manually just to make sure it stays good on repo level.

The new clone works better ! 👍 but a full rebuild still shows 2 dirty files :

image

image

You can see the details of the changes here

@cdietrich
Copy link
Contributor

cdietrich commented May 5, 2024

this seems to be the inverse change of
https://github.com/eclipse/xtext/pull/3022/files
maybe something got fixed in jdt again

which target platform did you select in oomoph? xtext-latest/2024-06?

@pbrossel
Copy link

pbrossel commented May 5, 2024

@cdietrich

which target platform did you select in oomoph? xtext-latest/2024-06?

If I remember well, I selected 'latest' when installing 2 weeks or so ago : 2024-03 was the latest at that time.

image

The currently starting IDE still shows 2024-03.

image

This is the target platform setting :

image

But /org.eclipse.xtext.contributor/Xtext.setup shows 2024-06 which seems to be right.

image

I guess I have to do the setup again, or can I ask oopmh to do it ?

@merks
Copy link
Contributor

merks commented May 5, 2024

Try Help -> Perform Setup Tasks. That should update everything, including the installation.

@pbrossel
Copy link

pbrossel commented May 5, 2024

Thanks for the hint.
I ran it, it did a couple of updates, but I still see 2024-03 when starting the IDE and the installation details still show :

image

Do I have to switch the active target platform to xtext-latest instead of Modular Target?

image

And a clean rebuild of project org.eclipse.xtend.core does not change to the right version -> #2778 (comment)

image

Note : Changing the active target platform does not seem to be a good idea :

image

@LorenzoBettini
Copy link
Contributor

The version of your eclipse and the version of the target platform are two different things. The version of eclipse is the one you selected when you select the distribution in oomph, the target platform is the one you select as an xtext property. The target platform is used for compiling and running tests.

@merks
Copy link
Contributor

merks commented May 5, 2024

You guys need an oomph configuration that chooses the latest product version so that things always stay up to date. Open an issue and I will help. But I have no time this week.

@pbrossel
Copy link

pbrossel commented May 5, 2024

@LorenzoBettini Yes, I agree. But I got 2 dirty files when doing a full rebuild i.e. compiling because of having the wrong eclipse version (if I understood @cdietrich right : #2778 (comment)).

It's not a problem for the issue I'm currently working on #2337 (comment).

Not easy for newcomers like me to guess the steps to do to catch up with the latest changes in the project.
Many thanks to all for the help.

@LorenzoBettini
Copy link
Contributor

@merks do you mean an issue here in Xtext?

@pbrossel I think @cdietrich referred to the target platform, which you can update either with Oomph by selecting the latest one or by opening the xtext-latest.target in Eclipse, wait for resolution, and set it is as current (or choose reload).
Concerning the running IDE, IIRC, you have to choose either the "Latest" (currently 2024-06) or "Latest Released" (currently 2024-03). Once you choose that, you cannot switch it: upon updating, it will update your Eclipse according to the stable (latest released) or the current milestone (latest). Is that right, @merks ? If you want to switch, you have to run Oomph from scratch.

Concerning the steps to catch up with the latest changes, I don't think there's a real workflow but to update now and then. ;) Now and then, "dirty" files like the two you mentioned can show up. Since you already have the latest Xtend in Eclipse, it should be enough to update the target platform. I had never seen the warning from PDE. You might try to ignore that.

@merks
Copy link
Contributor

merks commented May 6, 2024

You can use Navigate -> Open Setup -> Installation and via the properties view can switch to any product version:

image

Other projects have a configuration:

This allows you to determine/specify exactly which product and which version is to be installed, and can specify variable choices so the user can't make mistakes and the user needs to read fewer instructions.

It's quite simple:

https://raw.githubusercontent.com/eclipse/birt/master/build/org.eclipse.birt.releng/BIRTConfiguration.setup

@cdietrich
Copy link
Contributor

cdietrich commented May 6, 2024

i still wonder what jdt change causes the last problem.
i dont see the problem in m1

e.g. eclipse-jdt/eclipse.jdt.core@b2ac1e4

@pbrossel
Copy link

pbrossel commented May 6, 2024

@merks I changed the IDE version successfully. Thanks for the advice.
This setup has to be executed manually as soon as the next 'latest' comes out, right ?

image

Should I change the jdk settings too ? I had to replace it because of 'Failed to init ct.sym' errors, but do not remember where I changed it.

image

Oomph just allows to select a jre, not a jdk.

image

The whole discussion was very interesting and I learned a lot.
Automating the setup would nevertheless make life easier...

@cdietrich I can confirm that the 2 dirty files have disappeared after a clean rebuild.

@LorenzoBettini Did you find some time to look into this ?

I moved to my fork by replacing the contents of xtext-main\git\xtext : Is this the right approach ?

@LorenzoBettini
Copy link
Contributor

@merks I changed the IDE version successfully. Thanks for the advice. This setup has to be executed manually as soon as the next 'latest' comes out, right ?

If you perform setup tasks (not from scratch), it will update your IDE.
In general, you must perform setup tasks if you want to stay up to date.

A small warning: using the "latest" in your IDE (instead of "latest released") might expose you to some bugs introduced in the currently developed Eclipse or one of its plugins (e.g., #3018).

Oomph just allows to select a jre, not a jdk.

You can manually add a JDK to let Oomph know about that.
IIRC, it will automatically detect system JDK installations.

The whole discussion was very interesting and I learned a lot. Automating the setup would nevertheless make life easier...

well... it's automated... it's the small initial configuration that requires a few steps ;)

@cdietrich I can confirm that the 2 dirty files have disappeared after a clean rebuild.

Great! :)

@LorenzoBettini Did you find some time to look into this ?

I moved to my fork by replacing the contents of xtext-main\git\xtext : Is this the right approach ?

sorry, I had missed that.

Your solution is a bit... "brutal"... ;)

in general, not specific of Xtext, you first fork the github project, and then, during the Oomph setup, you specify the URL of your fork, instead of the main one.

Alternatively, if you cloned the main one, you can add your fork as an additional Git remote (either from the command line or from Egit); of course, no need to clone it from scratch, just as a remote. Of course, when you push, you have to push to the remote of your fork.

In general, it's good to have also the main repository as a remote in order to easily keep your fork up to date with the main one.

@pbrossel
Copy link

pbrossel commented May 7, 2024

@LorenzoBettini Thanks a lot! All open questions got answered 👍 !
I will follow your advice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants