View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0001352 | FreeCAD | Bug | public | 2014-01-11 17:53 | 2021-11-05 16:43 |
Reporter | jobermayr | Assigned To | |||
Priority | normal | Severity | major | Reproducibility | always |
Status | new | Resolution | open | ||
Platform | Linux | ||||
Product Version | trunk | ||||
Target Version | 0.20 | ||||
Summary | 0001352: DOS line endings makes applying patches on Linux really hard | ||||
Description | I 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 | |||||
|
Btw. it seems to be an easy task: https://help.github.com/articles/dealing-with-line-endings |
|
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 |
|
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 ... |
|
i don't even get to download the patch. <summary>Authentication required</summary> |
|
You could ask the packman build service guys to invoke "git am" with the parameter --keep-cr or "diff" with "--binary" |
|
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? |
|
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. |
|
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 |
|
@shoogen Does this workaround need to be documented somewhere and then we can close this ticket? |
|
Related forum threads: http://forum.freecadweb.org/viewtopic.php?t=15443 http://forum.freecadweb.org/viewtopic.php?t=19866 <-Active |
|
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. |
|
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). |
|
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? |
|
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. |
|
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. |
|
Linking 0003206 |
|
@wmayer do you think this ticket still has merit ? |
|
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. |
|
@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. |
|
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? |
|
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 |
|
This ticket has been migrated to GitHub as issue 5578. |
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 |