View Issue Details

IDProjectCategoryView StatusLast Update
0001352FreeCADBugpublic2021-11-05 16:43
Reporterjobermayr Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status newResolutionopen 
PlatformLinux 
Product Versiontrunk 
Target Version0.20 
Summary0001352: DOS line endings makes applying patches on Linux really hard
DescriptionI must apply patches on pmbs via "patch --merge -p1 <$PATCH" instead of just "patch -p1 <$PATCH" due to silly DOS line endings:
https://pmbs.links2linux.de/package/show?package=FreeCAD&project=Extra
Tags#tobeclosed
FreeCAD Information

Relationships

related to 0003206 assignedKunda1 Project Tools & Websites TravisCI to run code linting to report trailing whitespace/tabs + LF line endings 

Activities

jobermayr

2014-01-12 15:10

reporter   ~0004052

Btw. it seems to be an easy task:
https://help.github.com/articles/dealing-with-line-endings

shoogen

2014-01-12 18:28

developer   ~0004060

Did i get your right? You proposed to normalize the whole repo to LF line endings?
http://stackoverflow.com/questions/1510798/trying-to-fix-line-endings-with-git-filter-branch-but-having-no-luck/1511273#1511273

jobermayr

2014-01-12 19:14

reporter   ~0004062

Please try to apply https://pmbs.links2linux.de/package/view_file?expand=1&file=0001-Add-support-for-different-libdir.patch&package=FreeCAD&project=Extra

1. git am $file # fails
2. git reset --hard origin/master
3. patch -p1 <$file # fails
4. git reset --hard origin/master
5. patch --merge -p1 <$file # works
6. git commit -a --date="Mon, 2 Dec 2013 22:21:02 +0100" --author="Johannes Obermayr <johannesobermayr@gmx.de>" -m "Add support for different libdir."
7. git format-patch origin/master
8. git reset --hard origin/master
9. git am 0001-Add-support-for-different-libdir.patch # fails because of silly DOS line endings

So if you have another fix please let it know ...

shoogen

2014-01-12 20:07

developer   ~0004065

i don't even get to download the patch. <summary>Authentication required</summary>

shoogen

2014-01-12 20:20

developer   ~0004066

Last edited: 2014-01-12 20:25

You could ask the packman build service guys to invoke "git am" with the parameter --keep-cr
or "diff" with "--binary"

jobermayr

2014-01-12 21:16

reporter   ~0004067

Since I stepped in and am the de facto maintainer of FreeCAD on Packman I can tell you that %patch macro from openSUSE (main distro) applies patches via "patch $level -fuzz=0" also Fedora and some other RPM based Linux distros are doing it this way.

JFYI: None of the other packages on build.opensuse.org and pmbs.links2linux.de makes problems in the way FreeCAD makes (not FHS conform, RPATH issues, silly DOS line endings, ...)
Hacks and workarounds as well as hacks and workarounds to get them applied are needed to fulfill minimal requirements to get it working and accepted ...
Proper fixing is not possible in just a few minutes because jumping between c/c++ and python is a hell ...

Fixing OCE to our needs was a laugher:
https://github.com/tpaviot/oce/issues/created_by/jobermayr?state=closed
https://pmbs.links2linux.de/package/view_file?expand=1&file=0002-OpenGl_GlCore12.hxx-undef-GL_VERSION_x_y-in-all-case.patch&package=oce&project=Extra

Btw. the download problem is (WONTFIX on pmbs):
https://github.com/openSUSE/open-build-service/issues/458
Maybe you can copy & paste it?

shoogen

2014-01-13 16:57

developer   ~0004074

IMHO adding a single commit to our repo that will remove all Carriage Returns has some drawbacks.
The most important is that it will render the "git blame" command useless and that will make the repo bigger.
I would prefer to orphan the commit that will remove all the CRs. But would break all branches that have diverged from master.
But I am just an ordinary contributer.
Maybe we can just remove the CRs in all places touched by your patches, so they will apply cleanly. This won't solve the problem, as it would have to be repeated for every patch you add (in the future). But it will provide time to think about a suitable solutions, whilst allowing you to continue using PMBS.
And as this bug flagged as 'urgend' I think that it might be worth it.

shoogen

2014-01-19 13:26

developer   ~0004103

Last edited: 2014-01-19 13:41

this ticket is one week old. I see it is still major issue, but don't think that it is urgent anymore, sine there seems to be a workaround in place.

BTW: when debugging line end ending issues, i would usually avoid copy & paste.
https://github.com/openSUSE/open-build-service/issues/458
openSUSE 13.1 was out the day you wrote that

Kunda1

2017-01-10 16:25

administrator   ~0007608

@shoogen Does this workaround need to be documented somewhere and then we can close this ticket?

Kunda1

2017-01-17 04:05

administrator   ~0007829

Related forum threads:
http://forum.freecadweb.org/viewtopic.php?t=15443
http://forum.freecadweb.org/viewtopic.php?t=19866 <-Active

blacey

2017-03-03 22:06

developer   ~0008536

To me, this is something that @wmayer or @yorik should do because it will create a broad swath commit across the repo. I think with a little bit of communication, this can, and should, be done quite easily.

ian.rees

2017-03-03 22:50

developer   ~0008537

I think that if we're going to do a massive line-ending-change (my vote is to not to), it would be best to do it using something like filter-branch that rewrites history so we don't have a commit that touches every line in the project. We also need to have a process in place to make sure that the line endings stay consistent.

It will be a pain either way, but with the rewrite option, pain goes away once everyone's dev branches are based off the "new" history. Otherwise, it's always going to be more difficult to understand history from before the line ending fix.

Someone would need to come up with the process for moving the project to the new line ending style, and to handle a few common scenarios (like how to merge in branches that were made from the history with the wrong line endings).

yorik

2017-03-04 00:22

administrator   ~0008538

Won't it be an endless struggle, that even if we unify everything, very soon new commits will be added with "wrong" file endings again and we'll need to do this forever?

ian.rees

2017-03-04 02:50

developer   ~0008539

yorik - I think you're right. To keep line endings nice, we would also need a system to screen changes. That system might be nice, because it would help avoid individual files with both types of line endings.

As I understand the original issue above, unifying line endings to be "Unix" type isn't really going to solve the problem. The issue seems to be that the patches have line endings that aren't consistent with the file being patched. So, if we standardise on Unix then it makes creating good patches easier on Linux/MacOS, but harder on Windows.

blacey

2017-03-04 06:38

developer   ~0008540

The "endless struggle" could be averted through two methods: 1) add the line ending config to the .gitattributes file and 2) Add checks to Travis.

If everything is configured properly, all operating systems should conform to the line ending config while modifying files within the FreeCAD git repo. If someone goes astray, Travis can catch it while validating the Pull Request.

Kunda1

2017-10-19 13:12

administrator   ~0010331

Linking 0003206

Kunda1

2019-09-05 20:27

administrator   ~0013539

@wmayer do you think this ticket still has merit ?

wmayer

2019-09-10 11:50

administrator   ~0013564

In >99% of all cases we get contributions via GH (or Gitlab) and very rarely real patch files. So, patches with incompatible line endings is not a big issue any more.

But I would still leave this open because also on GH it happens from time to time that the line endings of a file has completely changed while only a single line has been modified by the author. This complicates the review, then.

chennes

2021-09-15 16:06

administrator   ~0015925

@wmayer, since there is no action to be taken on anyone's part, except to make sure your PRs don't mess up all the line endings, I suggest we close this. It's not a bug at this point, as much as "how to be a good open source developer" advice that would best be suited to a statement on the Wiki.

wmayer

2021-09-17 14:57

administrator   ~0015945

We haven't had any serious issues with line endings in the last couple of years or got patches that couldn't be merged easily. Also if a PR would change all line endings I found the trick to add ?w=1 to the url to ignore white spaces changes when reviewing the code.

@yorik what do you think?

ferdymercury

2021-11-05 16:43

reporter   ~0016015

Suggestion: one could mark the "big" commit changing all the line endings in a .git-blame-ignore-revs file, so that it can be easily 'hidden' from history, but does not make a mess with rewriting history and branches. This is also useful if one wants to use auto-formatters to reformat old code into a specific consistent style.

See:
https://github.community/t/support-ignore-revs-file-in-githubs-blame-view/3256
https://michaelheap.com/git-ignore-rev/
https://gitlab.com/gitlab-org/gitlab/-/issues/31423

yorik

2022-03-03 13:55

administrator   ~0016413

This ticket has been migrated to GitHub as issue 5578.

Issue History

Date Modified Username Field Change
2014-01-11 17:53 jobermayr New Issue
2014-01-11 18:41 shoogen Tag Attached: git
2014-01-12 15:10 jobermayr Note Added: 0004052
2014-01-12 18:28 shoogen Note Added: 0004060
2014-01-12 19:14 jobermayr Note Added: 0004062
2014-01-12 20:07 shoogen Note Added: 0004065
2014-01-12 20:20 shoogen Note Added: 0004066
2014-01-12 20:23 shoogen Tag Attached: packman build service
2014-01-12 20:25 shoogen Note Edited: 0004066
2014-01-12 21:16 jobermayr Note Added: 0004067
2014-01-13 16:57 shoogen Note Added: 0004074
2014-01-19 13:21 shoogen Priority urgent => normal
2014-01-19 13:26 shoogen Note Added: 0004103
2014-01-19 13:35 shoogen Note Edited: 0004103
2014-01-19 13:41 shoogen Note Edited: 0004103
2017-01-10 16:25 Kunda1 Note Added: 0007608
2017-01-17 04:05 Kunda1 Note Added: 0007829
2017-03-03 22:06 blacey Note Added: 0008536
2017-03-03 22:50 ian.rees Note Added: 0008537
2017-03-04 00:22 yorik Note Added: 0008538
2017-03-04 02:50 ian.rees Note Added: 0008539
2017-03-04 06:38 blacey Note Added: 0008540
2017-03-24 16:22 Kunda1 Tag Detached: packman build service
2017-03-24 16:22 Kunda1 Tag Detached: git
2017-10-19 13:10 Kunda1 Relationship added related to 0003206
2017-10-19 13:12 Kunda1 Note Added: 0010331
2019-09-05 20:27 Kunda1 Note Added: 0013539
2019-09-05 20:27 Kunda1 Tag Attached: #tobeclosed
2019-09-10 11:50 wmayer Note Added: 0013564
2021-02-06 06:50 abdullah Target Version => 0.20
2021-09-15 16:06 chennes Note Added: 0015925
2021-09-17 14:57 wmayer Note Added: 0015945
2021-11-05 16:43 ferdymercury Note Added: 0016015