12 ms·

When it comes to Git history, less is more

250 points by maximilianroos 5y ago

nailer 5y ago
Storing changes as text is a fundamentally bad idea. Not only does it make merge conflicts more likely to happen, as transforms (me adding a function, you renaming something I use in the function) more likely to occur, it also means that formatting is committed and discussed rather than being a matter of personal preference. Hopefully the next item in this RCS CVS Subversion Git chain is just storing ASTs and transforms on top of them so we can spend less time fixing basic conflicts and discussing formatting.
- iveqy 5y ago
  The way we store data is not the same as using that format to solve conflicts. Take a look at semanticmerge.com
- _ix 5y ago
  I've seen the suggestion of VCS/SCM storing ASTs rather than plaintext for a decade now. Are you aware of any projects that are trying to address this?
  - arp242 5y ago
    I heard Unison[1] does this, or something like it. Never looked at it myself, so not an endorsement. Just came up in the Lobsters discussion[2] on this last week where someone mentioned it. [1]: https://www.unisonweb.org/docs/tour/ https://www.unisonweb.org/docs/tour/ [2]: https://lobste.rs/s/b9pddy/when_it_comes_git_history_less_is_more#c_swwvjf https://lobste.rs/s/b9pddy/when_it_comes_git_history_less_is...
- luffapi 5y ago
  Not sure why you are downvoted. This is a genuinely interesting idea and seems like the basis for next gen change management.
  - morelisp 5y ago
    They are being downvoted for not understanding how git works. If you have a 3-way diff/merge tool for ASTs you can plug it into git and use it today, and you can use it on all existing branches and historical changesets. The "problem" is no one actually wants to resolve merges that way.
    - luffapi 5y ago
      There’s been plenty of times my code as been “lost” in a later commit because Git didn’t know it was the same thing.
      - morelisp 5y ago
        I don't understand how this relates to what I said without more details about what "lost" means, sorry. If you mean Git had issues finding some specific code motion to show in a diff, you can try one of the other diff algorithms, or adjust the threshold for rename/copy detection. AST-based differs would also suffer this issue; a "nice diff" is not a formal problem and does not have a universal solution. If you mean you once had a mis-merge that dropped some code you didn't want to drop, this won't go away with AST-based diffs. It will just happen at the token level instead of the line level. If you mean it's generally hard to deal with collapsing lots of branches with shared history, that's true but would also be true with AST-based approaches. This is the situation something like pijul could help with, but also raises all the other tradeoffs of snapshot vs changeset based approaches.
    - nailer 5y ago
      You’re still committing formatting, and our whole ecosystem of tools is focused around text. You’re proposing a workaround rather than fixing the problem.
      - morelisp 5y ago
        I don’t know, to me whether you store the AST as text or binary sounds like ignoring the problem. Why would that matter?
- morelisp 5y ago
  Git stores snapshots, not diffs, and therefore could just as well be considered storing ASTs. The trick is writing a useful diff/merge for them. Programmers also don’t think in ASTs, arguably even less so than lines. The problem is not as formal as it looks.
  - rwbhn 5y ago
    > Programmers also don’t think in ASTs, Citation needed
    - morelisp 5y ago
      T_PAAMAYIM_NEKUDOTAYIM
  - luffapi 5y ago
    There’s great irony in someone who’s name is “morelisp” saying that programmers don’t think in ASTs. Lisp syntax is the AST.
    - morelisp 5y ago
      Which also means the Lisp AST is not great to diff with. First, it's too weak - you need to at least recognize special top-level defun-style forms, or you'll generate some minimal diff between two totally different functions just because they both use the same cond pattern or whatever. Second, reader macros mean you can't really work on the source AST in the first place, unless you also teach the diff tool all your reader macros.
- pjc50 5y ago
  Does that mean git would have to support all known programming languages, and any language syntax change would require a backwards incompatible upgrade of the VCS?
  - nn3 5y ago
    As well as any known config file formats.
  - globular-toast 5y ago
    More likely git would support a common AST format/protocol and language tooling would be responsible for providing that.
  - nailer 5y ago
    Yes. Much like we do with editors today.
- _ix 5y ago
  In a sibling comment, I was wondering about projects that are trying ASTs. I haven't read deeply about it, but I recalled that [pijul][1] might be a way forward. [1]: https://pijul.com/manual/why_pijul.html#comparisons-with-other-version-control-systems https://pijul.com/manual/why_pijul.html#comparisons-with-oth...
  - morelisp 5y ago
    Pijul's patch algebra, and storing diffs rather than snapshots generally, makes it less amenable to these kinds of experiments than Git. Changing your diff/merge strategy would be akin to rewriting your entire project history.
- breck 5y ago
  In Tree notation the text and ast have the same shape. You can then have a semantic git https://arxiv.org/pdf/1703.01192.pdf https://arxiv.org/pdf/1703.01192.pdf
- ycombobreaker 5y ago
  Git stores more than just code, though. The tool already supports custom merge tools, so why not apply your favorite AST mergetool to the relevant source files? Your suggestion (what comes after git) is throwing out the baby with the bathwater.
geofft 5y ago
I agree with the author re Git as it exists right now, but this feels like an unsolved technical problem that ideally should be handled in your VCS than something to be worked around by social conventions not to do cleanups. git blame, for instance, has the "--ignore-revs" and "--ignore-revs-file" options which let you specify some commits that can be ignored. This is a little helpful for the sort of code spelunking the author does (which I do very frequently), but it's a manual process. There's no built-in convention to Git for what this file should be named and how to have git blame pick it up automatically. Moreover, you have to create the commit in order to know what its revision is, which means that in squash/rebase workflows, you have to land the style cleanup on the target branch and only then can you add it to the ignore list. One option that occurs to me is a special marker string to put in the commit message like "Git-Ignore-For-Blame: yes" or something. I'm not sure if there's a better way to do it. More generically, if the commit is only swapping out tabs and spaces, "git blame -w" ought to take care of it. For more involved reformatting, git diff and git merge both have "ignore-all-space" modes, which will handle things like line breaks. But really what you want for blames and cherry-picks, I think, is blames at the syntax level. The Git commands only know about lines - git blame runs on lines, git cherry-pick constructs diffs based on lines and tries to re-apply them, etc. But at the repository level there's nothing special about line breaks. Why can't we run git blame or cherry-pick on an AST, so that it operates regardless of formatting? Are there tools that do this? (They don't need to be built into git; a third-party tool ought to be able to do this just fine.)
- a-dub 5y ago
  i think what you (and the OP) are describing goes beyond making merges more robust to nonfunctional changes by using something like an AST representation. making text edits apply cleanly is one part of the problem, but as the OP notes, the harder part of these merges is actually when interfaces stay the same but behavior changes. i think that to solve that, you'd need some kind of static analyzer that could jump from changed lines/syntax to execution paths and then to diffs of them. i bet there are some cool papers out there on trying to do this... but, this is essentially a restatement of the halting problem, so building something that is guaranteed to be correct using only static analysis may be impossible. (but this does not preclude a solution that might be good enough)
foreigner 5y ago
On this topic, is there a JavaScript lint tool that integrates with Git to enable you to gradually apply style changes to code as it evolves? E.g. if the change is tabs-to-spaces the lint tool would require spaces in new or changed lines of code, but not complain about tabs in unchanged lines of existing code.
- jabo 5y ago
  You could have the linter only run on changed files in a precommit hook, instead of on all files.
  - foreigner 5y ago
    That would work at file level granularity, but I'd like it to work at line level.
    - masklinn 5y ago
      You can usually take the output of the lint tool and filter it by the lines modified by the patch (which you get from the diff). Few tools do it out of the box (in the python world flake8 supports diff filtering out of the box, and there's diff-cover which can filter analysis beyond just test coverage) but it's easy enough to do with a wrapper, especially if the linter has a configurable or even parseable (e.g. json) output option.
    - jabo 5y ago
      Found an ESLint plugin that will only run on staged changes: https://www.npmjs.com/package/eslint-plugin-diff https://www.npmjs.com/package/eslint-plugin-diff
  - surfmike 5y ago
    This still leads to frustrating behavior, where changing one line of a file might end up reformatting all of it (and this often obscures the actual change itself).
    - 5y ago
mananaysiempre 5y ago
On one hand, I agree that these are all good points. I haven’t ever played with kernel code, but I have tried to backport years-old Glibc commits and it’s an absolutely miserable experience because “minor” things like small-scale reformatting would frequently be bunched together with unrelated changes. They also maintained their configure script in version control and apparently even merged it manually, because I’ve found that at many points in its history it would be composed of pieces generated by different versions of autoconf. On the other hand, I don’t think that the conclusions of this story are as inevitable as it makes them seem. Much of the pain it describes is due to the fact that Git is, as it proudly itself, stupid: it doesn’t understand that your spacing or line breaking or bracketing changes are incidental to what the code is trying to accomplish. I’m not saying that structural or otherwise language-aware editing or source control is the silver bullet because I’m well aware that so far most attempts at it suck, but I think it’s important to remember that whitespace pedantry is to a large extent a tooling issue. Could a merge tool coupled less than completely to clang-format (or yapf, or black, or gofmt ...) use its settings to improve the merges, I wonder? (Tangentially related: Monticello, Paredit, the “skeleton syntax tree” idea from Dylan.)
- Ericson2314 5y ago
  Yeah absolutely. My personal measure for a the "cohesion-prowess" of a society is how much they screw around with plain text, opaque binary data, etc. vs take the time to properly create rich data structures for interfaces. Taking the time and deep breath to write nice interfaces rather than scrambling and putting all the effort into implementations ignoring the larger picture of how human endeavors compose seems very fundamental. It reminds me also of the slogan > A developed country is not a place where the poor have cars. It's where the rich use public transportation.
  - jolux 5y ago
    > > A developed country is not a place where the poor have cars. It's where the rich use public transportation This is brilliant, stealing it for my transit advocacy.
pdw 5y ago
> At a previous company, there was an “infamous” commit in our main repository. The commit was about 10 years old, and it replaced every tab with 4 spaces. If have commits like this, add the ids to a file `ignorerevs`, and then tell git about it: git config --local blame.ignoreRevsFile ignorerevs Then at least `git blame` will still give useful results. (This is a relatively new git feature, added a year or two ago.)
- catlifeonmars 5y ago
  Nice! Is there a way to check this into a repository (a la .gitigore)?
  - banana_giraffe 5y ago
    You can specify a file with of such commits with --ignore-revs-file, and of course check in that file. Naming that file .git-blame-ignore-revs seems to be a convention I've seen more than once. Still need to specify the file yourself, so hopefully someone can point out the missing magic to have git pull in .git-blame-ignore-revs on its own.
    - NegativeLatency 5y ago
      A shell alias?
    - karlding 5y ago
      That's what the blame.ignoreRevsFile [0] config option is for. [0] https://git-scm.com/docs/git-config#Documentation/git-config.txt-blameignoreRevsFile https://git-scm.com/docs/git-config#Documentation/git-config...
      - IshKebab 5y ago
        Yes but you can't check set git config from a repo unfortunately because some things in it would cause security issues. banana_giraffe was asking for a way to set it up automatically so users don't have to do anything.
prpl 5y ago
It would be cool if you could mark whitespace only changes at least with an option to smart-skip them with `git blame` or something
- hnra 5y ago
  Isn’t this possible with just the normal git blame -w? Or if you want to ignore specific commits there is ignore-revs.
cellularmitosis 5y ago
It is a shame that we don’t have better tools, and that we are still hand-editing text files in order to write programs. Imagine having an editor which automatically applied your local preferences around tabs/spaces, code formatting, variables up top vs nearest use, function definitions nested to minimize top-scope surface area vs all functions flat at top scope, etc etc etc And when you are done editing, all of these local changes are reversed and you submit the minimal possible diff. (and if we want to really talk pipe dreams, the dev only sees an AST editor and the underlying text is never even exposed in the first place) Our tools are so far behind mostly because everyone’s thinking is still chained to 1970’s hand editing text files mentality. This is the flying car which isn’t being worked on because everyone is still thinking about making better bicycles.
- spaetzleesser 5y ago
  Agreed. It seems silly that we still have to deal with things like tabs vs spaces or formatting in different ways. This should be handled by IDEs and editors.
- cjfd 5y ago
  AST editting would be kind of okay. But never better than kind of okay. Thoughtfully formatted text can be quite a bit nicer. Also, cities where one can get around on bicyles are much nicer than where one needs to use cars. Flying or otherwise. https://www.youtube.com/watch?v=ul_xzyCDT98 https://www.youtube.com/watch?v=ul_xzyCDT98 To summarize both points: what looks like progress superficially may actually not be progress at all.
- ajuc 5y ago
  The benefit is minimal, the task is hard, there's decades worth of tools that won't work, and it's easy to mess up introducing "impossible bugs" with behind-the-scenes transformations. Also - AST isn't THAT important when reading the code. Let's say I give you this: X (X X X X; X X X; X X) { X (X X X) { X(X X X X X) } } Do you know what this code does? How about this: for int i = 0 i < 10 i ++ if i % 2 printf " %d " , i I've used several graphical languages professionally (not AST-based, graph-based, but the problem remains) and the main problem was - structure wasn't fully describing what happens - the "meat" of the behaviour was still in text form and was hidden behind the pretty graphic form - in case of both of these languages the meat was in the names of the subprocesses called and in the mapping of process variables <-> subprocess parameters. And there were A LOT of these, so you couldn't show them at once on the same screen as the graphical view of the process. So programming with both of these languages was very frustrating - you had to click on each node and look through long lists of x:y substitutions to track how parameters flow through the system.
spankalee 5y ago
This isn't good general advice. The vast majority of projects aren't going to need to cherry pick commits to such old branches, and overly limiting style fixes and refactorings is a good way to ossify a code base.
- cypressious 5y ago
  This is exactly what the author is saying in the final paragraph.
- sharken 5y ago
  The same problems exist when you merge between multiple release branches, there is an inherent resistance to refactoring as it interferes with merges. To me the solution is to migrate to trunk based development and hide new features behind feature flags. The biggest issue with this transformation seems to be implementing feature flags in the code.
pansa2 5y ago
> The commit was about 10 years old, and it replaced every tab with 4 spaces. Reminds me of a project I worked on a while back - whenever you looked at the git history of a file, if you went back far enough you’d hit a dead end at the point where every source file was renamed from *.c to *.cpp.
- iveqy 5y ago
  —follow We have a few files at work that change name with every commit…
- deleted 5y ago
  [deleted]
- alkonaut 5y ago
  Git tracks changes across renames though right?
  - rocqua 5y ago
    I think git can be convinced to see a rename as a delete followed by a file creation.
  - iveqy 5y ago
    Not really. Git tracks content and not files. File renames doesn't exists in git. When git tell you that a file is renamed it's because an algorithm guesses that a file is renamed (if the content is similar).
    - alkonaut 5y ago
      Exactly, so any rename should be done without other edits in the same commit, so the content hashes exactly the same. Any bulk rename in a repo would satisfy that and be a non-issue.
      - morelisp 5y ago
        Of course, this is impossible in languages where file names and namespaces / structures must align. But nobody uses those, right?
CalChris 5y ago
What holds true for the Linux kernel may not hold true for other repos. In this case, the OP is supporting old Linux releases and remarking how naturally cherry picking can work. LLVM by comparison, doesn't have long-term stable releases. So this conservative approach doesn't necessarily apply for LLVM. So I like the analysis but I disagree with the generality.
- gumby 5y ago
  > So I like the analysis but I disagree with the generality. Any organization that releases software revisions has this issue. Yes, we do live in a world where much software is now subject to "continuous release" (either applications which are auto-updated to latest, on phones and some laptop OSes or web-only applications where the latest is pushed to deployment). And indeed the specific issue described here is much less important. However I suspect the majority of software in use is not like that. On the Enterprise side, OSes and applications are updated carefully; an ERP upgrade can be a major undertaking. All sorts of machine control software is ether never upgraded, with limited patching, or treated the same way as other enterprise software. Etc. The difference between the two is why there is both "git flow" and "GitHub flow" -- it's not like one is absolutely superior to the other.
- mojolozzo 5y ago
  Agreed! The author makes exactly this point towards the end of the post.
  - YesThatTom2 5y ago
    Maybe the author’s next piece should be about people not reading the entire article before they reply. Then there last paragraph would tell them he’ll ignore replies that don’t mention plaid.
muxxa 5y ago
git log -S'text of interest' has always served me better than blame. It can better jump file boundaries and find e.g. prior code that the code in question was copied from. I've always thought though that we need better conflict resolution that is code aware. And also better changeset specification, e.g. if a commit is equivalent to a s/foo/bar/ then this information should be included in the commit, or if a function signature has changed, then record that fact, rather than the dumb line by line diffs.
- Aeolun 5y ago
  We can probably diff AST’s? Someone just has to build it.
lilyball 5y ago
It sounds like the real advice here is “have a strong coding style guide that is rigorously enforced so nobody ever has to go make style fixes to existing code”.
daneel_w 5y ago
What was the point the author tried to get across with the first paragraph bringing up the large whitespace commit? It didn't add anything to the story, it just ended up as an initial distraction. Also, is he implying that thousands of commits (one for each file) would be a better way to convert that large code base? Why not solve this problem with the right tools, such as ignorerevs?
- ghshephard 5y ago
  The whitespace commit breaks cherrypicking. What seemed like a harmless change could cripple automated backports of fixes. I thought it was the entire crux of his story.
  - daneel_w 5y ago
    Yes but the whitespace commit - a change that unavoidably affects almost every single line of its target - is possibly the worst example one can come up with.
- looperhacks 5y ago
  I believe his point is that the commit shouldn't have happened at all, keeping the old style instead (or maybe applying the new style together with other changes ... Yuck. Don't do this) It kinda serves as a hook to describe why these changes are bad. I think it's a good example for the rest of the article.
  - daneel_w 5y ago
    I feel it's a very contrived example because there's no compromise to how such a change is to be done. If there's an argument to make it's about making the decision to switch whitespace tactic at all.
zmmmmm 5y ago
It reminds me that one of my grievances with whitespace significant languages (hello Python) is that I can't effectively just skip commits that are pure whitespace changes in code review. I am curious how people approach this ...
juped 5y ago
Have Linux maintainers forgotten how to use Git? You usually apply a bugfix patch to the commit that introduced the bug.

Storing changes as text is a fundamentally bad idea. Not only does it make merge conflicts more likely to happen, as transforms (me adding a function, you renaming something I use in the function) more likely to occur, it also means that formatting is committed and discussed rather than being a matter of personal preference. Hopefully the next item in this RCS CVS Subversion Git chain is just storing ASTs and transforms on top of them so we can spend less time fixing basic conflicts and discussing formatting.

iveqy 5y ago
The way we store data is not the same as using that format to solve conflicts. Take a look at semanticmerge.com
_ix 5y ago
I've seen the suggestion of VCS/SCM storing ASTs rather than plaintext for a decade now. Are you aware of any projects that are trying to address this?
- arp242 5y ago
  I heard Unison[1] does this, or something like it. Never looked at it myself, so not an endorsement. Just came up in the Lobsters discussion[2] on this last week where someone mentioned it. [1]: https://www.unisonweb.org/docs/tour/ https://www.unisonweb.org/docs/tour/ [2]: https://lobste.rs/s/b9pddy/when_it_comes_git_history_less_is_more#c_swwvjf https://lobste.rs/s/b9pddy/when_it_comes_git_history_less_is...
luffapi 5y ago
Not sure why you are downvoted. This is a genuinely interesting idea and seems like the basis for next gen change management.
- morelisp 5y ago
  They are being downvoted for not understanding how git works. If you have a 3-way diff/merge tool for ASTs you can plug it into git and use it today, and you can use it on all existing branches and historical changesets. The "problem" is no one actually wants to resolve merges that way.
  - luffapi 5y ago
    There’s been plenty of times my code as been “lost” in a later commit because Git didn’t know it was the same thing.
    - morelisp 5y ago
      I don't understand how this relates to what I said without more details about what "lost" means, sorry. If you mean Git had issues finding some specific code motion to show in a diff, you can try one of the other diff algorithms, or adjust the threshold for rename/copy detection. AST-based differs would also suffer this issue; a "nice diff" is not a formal problem and does not have a universal solution. If you mean you once had a mis-merge that dropped some code you didn't want to drop, this won't go away with AST-based diffs. It will just happen at the token level instead of the line level. If you mean it's generally hard to deal with collapsing lots of branches with shared history, that's true but would also be true with AST-based approaches. This is the situation something like pijul could help with, but also raises all the other tradeoffs of snapshot vs changeset based approaches.
  - nailer 5y ago
    You’re still committing formatting, and our whole ecosystem of tools is focused around text. You’re proposing a workaround rather than fixing the problem.
    - morelisp 5y ago
      I don’t know, to me whether you store the AST as text or binary sounds like ignoring the problem. Why would that matter?
morelisp 5y ago
Git stores snapshots, not diffs, and therefore could just as well be considered storing ASTs. The trick is writing a useful diff/merge for them. Programmers also don’t think in ASTs, arguably even less so than lines. The problem is not as formal as it looks.
- rwbhn 5y ago
  > Programmers also don’t think in ASTs, Citation needed
  - morelisp 5y ago
    T_PAAMAYIM_NEKUDOTAYIM
- luffapi 5y ago
  There’s great irony in someone who’s name is “morelisp” saying that programmers don’t think in ASTs. Lisp syntax is the AST.
  - morelisp 5y ago
    Which also means the Lisp AST is not great to diff with. First, it's too weak - you need to at least recognize special top-level defun-style forms, or you'll generate some minimal diff between two totally different functions just because they both use the same cond pattern or whatever. Second, reader macros mean you can't really work on the source AST in the first place, unless you also teach the diff tool all your reader macros.
pjc50 5y ago
Does that mean git would have to support all known programming languages, and any language syntax change would require a backwards incompatible upgrade of the VCS?
- nn3 5y ago
  As well as any known config file formats.
- globular-toast 5y ago
  More likely git would support a common AST format/protocol and language tooling would be responsible for providing that.
- nailer 5y ago
  Yes. Much like we do with editors today.
_ix 5y ago
In a sibling comment, I was wondering about projects that are trying ASTs. I haven't read deeply about it, but I recalled that [pijul][1] might be a way forward. [1]: https://pijul.com/manual/why_pijul.html#comparisons-with-other-version-control-systems https://pijul.com/manual/why_pijul.html#comparisons-with-oth...
- morelisp 5y ago
  Pijul's patch algebra, and storing diffs rather than snapshots generally, makes it less amenable to these kinds of experiments than Git. Changing your diff/merge strategy would be akin to rewriting your entire project history.
breck 5y ago
In Tree notation the text and ast have the same shape. You can then have a semantic git https://arxiv.org/pdf/1703.01192.pdf https://arxiv.org/pdf/1703.01192.pdf
ycombobreaker 5y ago
Git stores more than just code, though. The tool already supports custom merge tools, so why not apply your favorite AST mergetool to the relevant source files? Your suggestion (what comes after git) is throwing out the baby with the bathwater.

geofft 5y ago

I agree with the author re Git as it exists right now, but this feels like an unsolved technical problem that ideally should be handled in your VCS than something to be worked around by social conventions not to do cleanups. git blame, for instance, has the "--ignore-revs" and "--ignore-revs-file" options which let you specify some commits that can be ignored. This is a little helpful for the sort of code spelunking the author does (which I do very frequently), but it's a manual process. There's no built-in convention to Git for what this file should be named and how to have git blame pick it up automatically. Moreover, you have to create the commit in order to know what its revision is, which means that in squash/rebase workflows, you have to land the style cleanup on the target branch and only then can you add it to the ignore list. One option that occurs to me is a special marker string to put in the commit message like "Git-Ignore-For-Blame: yes" or something. I'm not sure if there's a better way to do it. More generically, if the commit is only swapping out tabs and spaces, "git blame -w" ought to take care of it. For more involved reformatting, git diff and git merge both have "ignore-all-space" modes, which will handle things like line breaks. But really what you want for blames and cherry-picks, I think, is blames at the syntax level. The Git commands only know about lines - git blame runs on lines, git cherry-pick constructs diffs based on lines and tries to re-apply them, etc. But at the repository level there's nothing special about line breaks. Why can't we run git blame or cherry-pick on an AST, so that it operates regardless of formatting? Are there tools that do this? (They don't need to be built into git; a third-party tool ought to be able to do this just fine.)

a-dub 5y ago
i think what you (and the OP) are describing goes beyond making merges more robust to nonfunctional changes by using something like an AST representation. making text edits apply cleanly is one part of the problem, but as the OP notes, the harder part of these merges is actually when interfaces stay the same but behavior changes. i think that to solve that, you'd need some kind of static analyzer that could jump from changed lines/syntax to execution paths and then to diffs of them. i bet there are some cool papers out there on trying to do this... but, this is essentially a restatement of the halting problem, so building something that is guaranteed to be correct using only static analysis may be impossible. (but this does not preclude a solution that might be good enough)

foreigner 5y ago

On this topic, is there a JavaScript lint tool that integrates with Git to enable you to gradually apply style changes to code as it evolves? E.g. if the change is tabs-to-spaces the lint tool would require spaces in new or changed lines of code, but not complain about tabs in unchanged lines of existing code.

jabo 5y ago
You could have the linter only run on changed files in a precommit hook, instead of on all files.
- foreigner 5y ago
  That would work at file level granularity, but I'd like it to work at line level.
  - masklinn 5y ago
    You can usually take the output of the lint tool and filter it by the lines modified by the patch (which you get from the diff). Few tools do it out of the box (in the python world flake8 supports diff filtering out of the box, and there's diff-cover which can filter analysis beyond just test coverage) but it's easy enough to do with a wrapper, especially if the linter has a configurable or even parseable (e.g. json) output option.
  - jabo 5y ago
    Found an ESLint plugin that will only run on staged changes: https://www.npmjs.com/package/eslint-plugin-diff https://www.npmjs.com/package/eslint-plugin-diff
- surfmike 5y ago
  This still leads to frustrating behavior, where changing one line of a file might end up reformatting all of it (and this often obscures the actual change itself).
  - 5y ago

mananaysiempre 5y ago

On one hand, I agree that these are all good points. I haven’t ever played with kernel code, but I have tried to backport years-old Glibc commits and it’s an absolutely miserable experience because “minor” things like small-scale reformatting would frequently be bunched together with unrelated changes. They also maintained their configure script in version control and apparently even merged it manually, because I’ve found that at many points in its history it would be composed of pieces generated by different versions of autoconf. On the other hand, I don’t think that the conclusions of this story are as inevitable as it makes them seem. Much of the pain it describes is due to the fact that Git is, as it proudly itself, stupid: it doesn’t understand that your spacing or line breaking or bracketing changes are incidental to what the code is trying to accomplish. I’m not saying that structural or otherwise language-aware editing or source control is the silver bullet because I’m well aware that so far most attempts at it suck, but I think it’s important to remember that whitespace pedantry is to a large extent a tooling issue. Could a merge tool coupled less than completely to clang-format (or yapf, or black, or gofmt ...) use its settings to improve the merges, I wonder? (Tangentially related: Monticello, Paredit, the “skeleton syntax tree” idea from Dylan.)

Ericson2314 5y ago
Yeah absolutely. My personal measure for a the "cohesion-prowess" of a society is how much they screw around with plain text, opaque binary data, etc. vs take the time to properly create rich data structures for interfaces. Taking the time and deep breath to write nice interfaces rather than scrambling and putting all the effort into implementations ignoring the larger picture of how human endeavors compose seems very fundamental. It reminds me also of the slogan > A developed country is not a place where the poor have cars. It's where the rich use public transportation.
- jolux 5y ago
  > > A developed country is not a place where the poor have cars. It's where the rich use public transportation This is brilliant, stealing it for my transit advocacy.

pdw 5y ago

> At a previous company, there was an “infamous” commit in our main repository. The commit was about 10 years old, and it replaced every tab with 4 spaces. If have commits like this, add the ids to a file `ignorerevs`, and then tell git about it: git config --local blame.ignoreRevsFile ignorerevs Then at least `git blame` will still give useful results. (This is a relatively new git feature, added a year or two ago.)

catlifeonmars 5y ago
Nice! Is there a way to check this into a repository (a la .gitigore)?
- banana_giraffe 5y ago
  You can specify a file with of such commits with --ignore-revs-file, and of course check in that file. Naming that file .git-blame-ignore-revs seems to be a convention I've seen more than once. Still need to specify the file yourself, so hopefully someone can point out the missing magic to have git pull in .git-blame-ignore-revs on its own.
  - NegativeLatency 5y ago
    A shell alias?
  - karlding 5y ago
    That's what the blame.ignoreRevsFile [0] config option is for. [0] https://git-scm.com/docs/git-config#Documentation/git-config.txt-blameignoreRevsFile https://git-scm.com/docs/git-config#Documentation/git-config...
    - IshKebab 5y ago
      Yes but you can't check set git config from a repo unfortunately because some things in it would cause security issues. banana_giraffe was asking for a way to set it up automatically so users don't have to do anything.

prpl 5y ago

It would be cool if you could mark whitespace only changes at least with an option to smart-skip them with `git blame` or something

hnra 5y ago
Isn’t this possible with just the normal git blame -w? Or if you want to ignore specific commits there is ignore-revs.

cellularmitosis 5y ago

It is a shame that we don’t have better tools, and that we are still hand-editing text files in order to write programs. Imagine having an editor which automatically applied your local preferences around tabs/spaces, code formatting, variables up top vs nearest use, function definitions nested to minimize top-scope surface area vs all functions flat at top scope, etc etc etc And when you are done editing, all of these local changes are reversed and you submit the minimal possible diff. (and if we want to really talk pipe dreams, the dev only sees an AST editor and the underlying text is never even exposed in the first place) Our tools are so far behind mostly because everyone’s thinking is still chained to 1970’s hand editing text files mentality. This is the flying car which isn’t being worked on because everyone is still thinking about making better bicycles.

spaetzleesser 5y ago
Agreed. It seems silly that we still have to deal with things like tabs vs spaces or formatting in different ways. This should be handled by IDEs and editors.
cjfd 5y ago
AST editting would be kind of okay. But never better than kind of okay. Thoughtfully formatted text can be quite a bit nicer. Also, cities where one can get around on bicyles are much nicer than where one needs to use cars. Flying or otherwise. https://www.youtube.com/watch?v=ul_xzyCDT98 https://www.youtube.com/watch?v=ul_xzyCDT98 To summarize both points: what looks like progress superficially may actually not be progress at all.
ajuc 5y ago
The benefit is minimal, the task is hard, there's decades worth of tools that won't work, and it's easy to mess up introducing "impossible bugs" with behind-the-scenes transformations. Also - AST isn't THAT important when reading the code. Let's say I give you this: X (X X X X; X X X; X X) { X (X X X) { X(X X X X X) } } Do you know what this code does? How about this: for int i = 0 i < 10 i ++ if i % 2 printf " %d " , i I've used several graphical languages professionally (not AST-based, graph-based, but the problem remains) and the main problem was - structure wasn't fully describing what happens - the "meat" of the behaviour was still in text form and was hidden behind the pretty graphic form - in case of both of these languages the meat was in the names of the subprocesses called and in the mapping of process variables <-> subprocess parameters. And there were A LOT of these, so you couldn't show them at once on the same screen as the graphical view of the process. So programming with both of these languages was very frustrating - you had to click on each node and look through long lists of x:y substitutions to track how parameters flow through the system.

spankalee 5y ago

This isn't good general advice. The vast majority of projects aren't going to need to cherry pick commits to such old branches, and overly limiting style fixes and refactorings is a good way to ossify a code base.

cypressious 5y ago
This is exactly what the author is saying in the final paragraph.
sharken 5y ago
The same problems exist when you merge between multiple release branches, there is an inherent resistance to refactoring as it interferes with merges. To me the solution is to migrate to trunk based development and hide new features behind feature flags. The biggest issue with this transformation seems to be implementing feature flags in the code.

pansa2 5y ago

> The commit was about 10 years old, and it replaced every tab with 4 spaces. Reminds me of a project I worked on a while back - whenever you looked at the git history of a file, if you went back far enough you’d hit a dead end at the point where every source file was renamed from *.c to *.cpp.

iveqy 5y ago
—follow We have a few files at work that change name with every commit…
deleted 5y ago
[deleted]
alkonaut 5y ago
Git tracks changes across renames though right?
- rocqua 5y ago
  I think git can be convinced to see a rename as a delete followed by a file creation.
- iveqy 5y ago
  Not really. Git tracks content and not files. File renames doesn't exists in git. When git tell you that a file is renamed it's because an algorithm guesses that a file is renamed (if the content is similar).
  - alkonaut 5y ago
    Exactly, so any rename should be done without other edits in the same commit, so the content hashes exactly the same. Any bulk rename in a repo would satisfy that and be a non-issue.
    - morelisp 5y ago
      Of course, this is impossible in languages where file names and namespaces / structures must align. But nobody uses those, right?

CalChris 5y ago

What holds true for the Linux kernel may not hold true for other repos. In this case, the OP is supporting old Linux releases and remarking how naturally cherry picking can work. LLVM by comparison, doesn't have long-term stable releases. So this conservative approach doesn't necessarily apply for LLVM. So I like the analysis but I disagree with the generality.

gumby 5y ago
> So I like the analysis but I disagree with the generality. Any organization that releases software revisions has this issue. Yes, we do live in a world where much software is now subject to "continuous release" (either applications which are auto-updated to latest, on phones and some laptop OSes or web-only applications where the latest is pushed to deployment). And indeed the specific issue described here is much less important. However I suspect the majority of software in use is not like that. On the Enterprise side, OSes and applications are updated carefully; an ERP upgrade can be a major undertaking. All sorts of machine control software is ether never upgraded, with limited patching, or treated the same way as other enterprise software. Etc. The difference between the two is why there is both "git flow" and "GitHub flow" -- it's not like one is absolutely superior to the other.
mojolozzo 5y ago
Agreed! The author makes exactly this point towards the end of the post.
- YesThatTom2 5y ago
  Maybe the author’s next piece should be about people not reading the entire article before they reply. Then there last paragraph would tell them he’ll ignore replies that don’t mention plaid.

muxxa 5y ago

git log -S'text of interest' has always served me better than blame. It can better jump file boundaries and find e.g. prior code that the code in question was copied from. I've always thought though that we need better conflict resolution that is code aware. And also better changeset specification, e.g. if a commit is equivalent to a s/foo/bar/ then this information should be included in the commit, or if a function signature has changed, then record that fact, rather than the dumb line by line diffs.

Aeolun 5y ago
We can probably diff AST’s? Someone just has to build it.

lilyball 5y ago

It sounds like the real advice here is “have a strong coding style guide that is rigorously enforced so nobody ever has to go make style fixes to existing code”.

daneel_w 5y ago

What was the point the author tried to get across with the first paragraph bringing up the large whitespace commit? It didn't add anything to the story, it just ended up as an initial distraction. Also, is he implying that thousands of commits (one for each file) would be a better way to convert that large code base? Why not solve this problem with the right tools, such as ignorerevs?

ghshephard 5y ago
The whitespace commit breaks cherrypicking. What seemed like a harmless change could cripple automated backports of fixes. I thought it was the entire crux of his story.
- daneel_w 5y ago
  Yes but the whitespace commit - a change that unavoidably affects almost every single line of its target - is possibly the worst example one can come up with.
looperhacks 5y ago
I believe his point is that the commit shouldn't have happened at all, keeping the old style instead (or maybe applying the new style together with other changes ... Yuck. Don't do this) It kinda serves as a hook to describe why these changes are bad. I think it's a good example for the rest of the article.
- daneel_w 5y ago
  I feel it's a very contrived example because there's no compromise to how such a change is to be done. If there's an argument to make it's about making the decision to switch whitespace tactic at all.

zmmmmm 5y ago

It reminds me that one of my grievances with whitespace significant languages (hello Python) is that I can't effectively just skip commits that are pure whitespace changes in code review. I am curious how people approach this ...

juped 5y ago

Have Linux maintainers forgotten how to use Git? You usually apply a bugfix patch to the commit that introduced the bug.