Hitting every branch on the way down

http://rachelbythebay.com/w/2024/04/29/pb/

by zdw • 2 years ago

160 140 2 years ago

rachelbythebay.com

datascienced• 2 years ago

I wont claim to understand C and the reason why <> is better than “”. I assume it is.

But the fact that a merge can have arbitrary changes in it always bothers me!

This is a case for rebase over merge if there are conflicts.

You could have a merge of 2 empty repo parents where the result is the complete source of the latest version of Kubernetes!

Groxx3• 2 years ago

Yep. Stuff like this is part of why I'm a rebaser.

Rebase is simple. Always. The end result is obvious and clear and can only be interpreted in one way.

Merge has lots of little sharp edges and surprises if you don't know every single tiniest detail.

Almost nobody knows it in that level of detail, so it's a terrible choice for interacting with anyone else. If you're on your own, sure, do whatever - many things are not built solo though.

zilti• 2 years ago

"But it is littering the commit history with useless commits!" is what I always hear

noirscape• 2 years ago

My personal preference is merge but using the --no-ff flag. That way you get all the advantages of a rebase (since all your original commits are rebased into the target branch) but you also get a merge commit to confirm that all those changes were a part of the same set of patches.

That can often help a lot to figure out why something ended up the way it did, but you also don't turn your entire history into a flat pile of disparate commits.

nvy• 2 years ago

I believe that the semantics of < > vs "" is actually compiler-dependent but on every compiler that matters, #including with angle brackets is the semantic for "the system header" whereas using quotes gives preference to files in your local source tree.

So for example if you #include <foo> then the compiler (actually the preprocessor, but whatever) looks in the system's standard location, whereas if you #include "foo" then it looks in the local tree.

ephimetheus• 2 years ago

I think that’s just the ordering though. “” will also end up searching the system paths, it will just check the local paths first.

ripe• 2 years ago

You are right; a good explanation of the rules is in the C FAQ [1], which points to a newsgroup posting by Kaz Kylehu [2].

I am posting the summary here, although please do read the original if you have time:

The most portable thing to do is to use "" for including files within your project, and to use <> only for implementation supplied files.

(Disclosure: I was one of the contributors to the C FAQ).

[1] https://c-faq.com/cpp/inclkinds.html

[2] https://c-faq.com/cpp/inclk.kaz.html

prerok• 2 years ago

What I find strange is that <> traditionally included system header files and "" included local files. They used different include paths, so you could have a header file in your sources with the same name as the system header file and then could control whether you are including one or the other based on using <> or "".

Anyway, I thought the distinction was lost in later compilers in favor of a single include path and then just taking the first file found when looking at potential matches through include path.

It seems the author of that merge thought the same thing. So, the distinction is actually still used by compilers?

pavon• 2 years ago

With both GCC and Visual C++, the “” form first searches local paths and then system paths, while the <> form only searches system paths. Guess some BSDs are stricter about local paths.

paulddraper2• 2 years ago

But a rebased commit can also have arbitrary changes!

---

P.S. Any commit can have any change. Or no change.

A "commit" is a version...a message, a tree, some metadata, and 0 or more parents. In fact it's not even a change/diff/patchset per se. Though will often compare it against its assigned parents. If it has multiple parents, you'd have to choose which to compare. If it has zero parents, you can't compare against any parents.

ptsneves• 2 years ago

Yes, except git log will show all the commits that got into the branch, while with merge you need git log -m otherwise there are invisible commits(and diffs) in a pretty common workflow. I don’t know why this is the default behaviour.

Git log only shows one tree not parallel trees from the merge.

btilly4• 2 years ago

One idiot with rebase destroys history with no trace. I worked with such an idiot in a parallel team. I can't say how many weeks of work randomly got destroyed by said idiot.

I hate rebase on shared code I don't care how clean jt looks. Don't mess with history.

Feathercrown• 2 years ago

I wouldn't consider rebasing your own local commits on top of a more recent remote master to be messing with history in any meaningful way, and that's the most useful method of rebasing.

doix5• 2 years ago

I really don't understand how you can lose weeks of work. The person that would have done the force push would have the original commit in their reflog. ORIG_HEAD would be set.

Everyone else that had a copy of the repo would have had a copy of the "lost" commits.

I really cannot imagine how many things would have to go wrong for weeks of work to be lost.

arghwhat6• 2 years ago

One never rebases shared code. They rebase their own work branch. Messing with history of master/main/integration branches should be blocked.

Rebase is a necessary part of a workflow even if you like merge commits. You're severely missing out if interactive rebases are not part of your toolbox.

ansible• 2 years ago

Didn't you guys have filesystem backups of a shared git repository?

This is exactly what backups are for.

globular-toast• 2 years ago

Rebase cannot destroy "weeks of work". No git command can delete commits. Unless you have some insane garbage collection policy that is very far from any defaults. This is your fault for not understanding your tools.

thaumasiotes• 2 years ago

I wont claim to understand C and the reason why <> is better than “”. I assume it is.

That one's obvious, you can type <> and you can't type “”.

devjab• 2 years ago

Wouldn’t you have the same amount of merge conflicts with rebase? Especially if you don’t do it often, which you frankly also should with merge?

I have to admit that I never really understood the advantages of rebase, and what I mean by this is they I actually don’t understand how the dangers of rebase out-weighs any form of advantages. Especially because on of the major advantages of merge is that you can squash your local commit history when you submit it to your main branch.

What we do is that we tie every pull request to a relatively small feature task, and because we do this, we genuinely don’t care about the individual commits developers do. Which means they can commit really silly messages if they are heading to a meeting or if they are just tired at the end of the day. It also helps with them merging main into their branch often, because it doesn’t taint the history.

The biggest advantage we’ve seen, that maybe we didn’t expect, is that nobody ever fucks up our tree in a way that needs someone who actually understands git to solve. We’ve also locked down the use of force push so that is not available to anyone unless it’s absolutely needed. Part of the reason I set this up initially was to protect myself from me, but it’s been a good thing since.

But I’m actually curious if it’s wrong.

doix5• 2 years ago

Especially because on of the major advantages of merge is that you can squash your local commit history when you submit it to your main branch.

Squashing is in no-way limited to merging and is actually done by doing an interactive rebase. Nothing is stopping you from squashing without creating a merge commit. It's entirely separate.

If you're squashing everything anyway, what does merging even give you? Is your main branch just:

* merge B

* squashed commit B

* merge A

* squashed commit A

If you didn't merge, you'd have:

* squashed commit B

* squashed commit A

What we do is that we tie every pull request to a relatively small feature task, and because we do this, we genuinely don’t care about the individual commits developers do.

Except eventually there is a large feature task and then you end up with a giant commit that is annoying when git-bisecting.

But at the end of the day, these things only matter if you are spelunking through git history and/or using things like git bisect. If your git history is "write-only & rollback", then none of this stuff matters.

xorcist6• 2 years ago

advantages of merge is that you can squash your local commit history

No, it's the other way around. Squashing is a type of rebase.

Most workflows involve both. Merges can also be fast-forward merges, which are indistinguishable from rebases. Choosing between a rebase and a merge operation is often the wrong question to ask. The question is what state you wish the repository to end up in.

I’m actually curious if it’s wrong

Look at "git log". It is readable and easy to understand? It is obvious why each commit was made, and why alternative solutions were turned down?

Are you able to use "git bisect" to track down problems?

Then you're doing it right. If not, think about what a functional commit log would look like and how you would get there. Working together is culture, and what type of merges you decide to use is just a tiny part of that culture.

arghwhat6• 2 years ago

All but the first commit has parents. All commits point to the state of the file tree at that point.

A "merge commit" is nothing more than a commit claiming any number of parents greater than one. It is still its own file tree reference that decides how the tree looks, and nothing dictates that it should be related to the parents.

kccqzy• 2 years ago

You are right that conceptually this is okay. But it is a UI problem that commands the author tried didn't manage to show the difference between the merge commit against any of its parents.

robin_reala• 2 years ago

Technically you can have multiple first-commits in a Git repository. For example, Linux had 4 initial commits in 2017: https://www.destroyallsoftware.com/blog/2017/the-biggest-and...

tsimionescu4• 2 years ago

If that were true, then git log -p would have worked. The reality is that merge commits are treated differently from other commits by many parts of git. Saying that they are "just a commit with multiple parents" gives people the wrong impression.

Git is more than the data structure backing it. And many parts of git make all sorts of assumptions that treat things that are more or less identical in the data model as actually being different. Tags are not the same thing as branches, for example, even though they are stored in virtually the same way.

red_admiral• 2 years ago

The <> version searches the library path (usually /usr/include/* but can be modified with flags) whereas the "" searches the current working directory.

Gibbon1• 2 years ago

But the fact that a merge can have arbitrary changes in it always bothers me!

After that xy thing where they were trying to install a back door having changes that are hidden like this is a big red flag.

In fact changing include <something.h> to include "something.h" with a hidden commit like this isn't a red flag it's a big rotating alarm with a siren. Someones trying set things up to include malicious code via a faked system lib.

saagarjha4• 2 years ago

Sadly, not all of us can live in the tech equivalent of Bond films. There is only so many xz backdoors to go around.

bananskalhalk• 2 years ago

    $ git show  d85c9944c55fb38f4eae149979a0f680ea125ecb  | wc -l
    11067
    $

From `man git-log`: "Note that unless one of --diff-merges variants (including short -m, -c, and --cc options) is explicitly given, merge commits will not show a diff, even if a diff format like --patch is selected, nor will they match search options like -S. The exception is when --first-parent is in use, in which case first-parent is the default format."

Presumably the author would have been happier using the -m-flag in addition to -p.

thriftwy3• 2 years ago

And that's one of the reasons why I advocate against merges in the codebase on every project I work for and in every HN thread where the topic of merges is mentioned.

baud147258• 2 years ago

That might reveal the depths of my ignorance of Git, but how do you manage moving changes from one branch to the other if you don't use merge? Edit: continuing to read the discussion, it seems it's rebase? I have some reading to do

jeffrallen• 2 years ago

This reminds me of a comment my new boss made, "you like learning on hard mode". He meant that instead of following doc to learn, I want to go find out how it works from first principles and then follow the docs, maybe improving them, based on what I saw from "beneath them" looking up.

iainmerrick4• 2 years ago

I like to think of that as "actually learning"

xorcist6• 2 years ago

That's just regular "learning". It's just that for some people it's a bit out of fashion.

BirAdam• 2 years ago

If more people did this (across many different industries) life would likely be substantially better. However, humans always optimize for the wrong things.

saghm2• 2 years ago

I told it to install "protobuf" since I use that library in my build tool. That actually installed "protobuf-24.4,1" which is some insane version number I'd never seen before. All of my other systems are all running 3.x.x type versions.

I was curious about this, so I took a look at the list of protobuf releases[0] and they're...confusing, to say the least. Chronologically, the most recent tags at the time I write this comment are:

v5.27.0-rc1 v3.27.0-rc1 v27.0-rc1 v27-dev v26.1 v5.26.1 v3.26.1 v26.1 v26.0 v5.26.0 v3.26.0

Does anyone here happen to know what's going on here? As best I can tell, they're simultaneously supporting 3 major versions while keeping their minor and patch versions in lockstep, and then having one major version be implicit?

I can almost imagine a scenario where they started out with just major and minor version and then realized they wanted to make breaking changes, which led them down the path of adding a third separate number to the versions, but if they already were going down the path of assuming "wider" versions are newer , why not just stop using version numbers with only major-minor and instead just add a 0 or 1 to the front of all of the continuations of that branch? Also, why synchronize every single minor and patch version between all three major versions? I can understand why it might be useful to continue providing support for multiple major versions at the same time, but I'd expect that _sometimes_ there might be a bug or something in only one of them, and pushing out a release of the other two that don't contain any changes would be pretty strange.

[0] https://github.com/protocolbuffers/protobuf/tags

maeln• 2 years ago

It's because they changed the versioning format: https://github.com/protocolbuffers/protobuf/releases?page=5 / https://protobuf.dev/news/2022-05-06/

But I suppose old version still receive bugfixes.

BobbyTables22• 2 years ago

If only they had a language agnostic way of encoding version information…

Perhaps someone wrote a library for that…

saghm2• 2 years ago

I don't understand how either of those links clarify anything; the former shows that they had a version 3.y.z and a version 20.y, and the later shows a change from 3.20.x to 4.21.x, but none of it gives any explanation for why a new major version should "inherit" the old minor version instead of restarting from 0, or why they kept around `x.y` when updating to use `x.y.z` instead of just continuing the existing branch as `0.x+1.y` or `1.x+1.y`. If anything, the fact that they already seem to assume that new major versions should always inherit the minor version would make it _more_ consistent than having one "special" branch that has "narrower" versions.

ori_b• 2 years ago

I think protobuf might just be a performance art piece, exploring the question of how much complexity it's possible to insert into a simple concept.

BobbyTables22• 2 years ago

Indeed. Having used it and examined the resulting binary formats it uses, I don’t get it. Google makes some nice things, but protobuf isn’t one of them.

IIRC (5 yes ago), it was somewhat self describing with respect to struct offsets but had no (data format) versioning or type information. Never understood that design choice, even in mixed endian+architecture environments.

Ends up being horribly inefficient for small/one-time instances, and not all that great for client/server use.

We were looking at it to get away from C structs and handmade serialization — only to unexpectedly realize that our methods were still better. (We genuinely did not want this outcome!). JSON/yaml were not options for other reasons.

Protobuf is the modern realization of “the king has no clothes”.

bitcharmer• 2 years ago

That depends. In low latency environments you will see some protobuf but not as much as you'd think. SBE is quite common with the odd exception of some bespoke serialization protocols with delta compression and such.

microtherion2• 2 years ago

Rather than the sed post-processing, the author could also have used -iquote for the place where protobuf is installed, which makes it findable by quoted includes.

lloydatkinson• 2 years ago

Is this a common solution or documented in obvious places?

It wasn’t until I just read her article that I’d even considered some systems/distros doing weird things like rewriting C include syntax for questionable reasons.

What a terrible thing to deal with, simply frustrating.

microtherion2• 2 years ago

It is documented in the gcc and clang documentation. It's also described in the gcc manpage, but not the clang one.

I don't know how "common" it is, but when dealing with third party code, I run across <> / "" confusion quite commonly, so when that's a significant part of your job, you'll probably stumble upon this flag eventually.

zokier• 2 years ago

I don't want to victim blame too much, but this line stood out to me

There's no "body" to this commit. It's just a "Merge:" and two other commits

Commits are snapshots of repository state, and merges "obviously" have differences from its parents. So not having "body" for a commit is bit nonsensical in git (yes, technically you can make empty commits but that's a special case). These sort of things are where having good mental model of git is useful.

As I have my share of hairy merges, it is pretty intuitive that merge commits can, and in many cases need to, have changes that are not part of either parent.

Maybe something like pijul (/darcs) would handle things differently here, but I believe that merges are fundamentally difficult problem.

planede• 2 years ago

Merges of course should have changes, but IMO they shouldn't have changes that are not resolving conflicts (either actual conflicts marked by git, or conflicts that manifest in failing tests, etc...). An entirely unrelated change stuffed into a merge commit is inappropriate.

thriftwy3• 2 years ago

That's why reasonable projects limit their usage to the smallest reasonable scope, and use rebase/squash. You don't have to adopt a hard problem.

It actually escapes me why Linus decided to make git a merge-first VCS in the first place. There aren't many projects which are more linear in nature than Linux kernel.

nebulous12• 2 years ago

Merging a branch is one of the "special cases" where an empty merge commit can be used. Git will use fast forward by default but if you want to preserve the history as a separate branch you can use an empty merge commit instead.

tsimionescu4• 2 years ago

So not having "body" for a commit is bit nonsensical in git (yes, technically you can make empty commits but that's a special case). These sort of things are where having good mental model of git is useful.

This isn't a problem with the author or her mental model, it is a problem with `git log -p`. The output she is describing is exactly how merge commits show up there, with no other flags.

rob742• 2 years ago

So, someone at some point in some commit that we will never see because it got squashed with other commits thought it would be cooler to use absl::StrCat() instead of the "+" operator, and in the process of doing that, they went "what's this useless code using angled brackets instead of quotes?! It works with quotes too, let's delete it!". Or maybe that part was difficult to test, so they simply deleted it to increase test coverage? Guess we will never know, but still, open source is now a bit shittier because of it. Thanks, anonymous clueless developer!

    --  std::string left = "\"";
    --  std::string right = "\"";
    --  if (use_system_include) {
    --    left = "<";
    --    right = ">";
    --  }
    --  return left + name + right;
    ++  return absl::StrCat("\"", basename, "\"");

nebulous12• 2 years ago

Am I misunderstand or is this not on whoever abused the merge commit, whether they made the change personally or not.

nerdponx• 2 years ago

It's a shared responsibility.

xorcist6• 2 years ago

Why do you think it was squashed? This article is clear about this being a merge commit. This person got a merge conflict and decided that this was the best way to fix it. It probably worked on their machine. Perhaps not necessarily the optimal fix when you have thousands of users depending on this code, but what do I know?

rob742• 2 years ago

Ok, maybe I'm misunderstanding the line "There's no explanation or other context. Presumably that all got squashed out when it was exported from whatever they use internally." - I was thinking about git squash, but it could have also been some other step in the process of transferring the code from Google's internal systems to GitHub.

Sharlin• 2 years ago

But according to the article, both of the parents contained the same old code. Could be that it was just collateral damage in a larger conflict, but still it seems that someone used the merge commit to make an off-topic, ad-hoc change that was not only superfluous but code-breaking.

Applejinx• 2 years ago

It's such a spectacular Chesterton's Fence. Always so frustrating dealing with people who're like "from where I'm standing…" and then they go and make things better, meaning 'more abstract and/or fewer keystrokes'.

plq2• 2 years ago

In the chromium repository, the use of angle brackets in #include statements is banned -- they only use double quotes. They also don't use any system headers per se since their flavor of clang comes with their flavor of libcpp vendored in.

So if the chromium repo is representative of the state of C++ in rest of Google, they ditched it silently like this probably because it's so natural to them :)

RHSeeger3• 2 years ago

So, instead of using

<> means system headers and "" means project headers (my naive understanding of the difference)

they just convert everything to a project header? That seems bonkers to me. It is intentionally removing useful information.

pjc506• 2 years ago

Reminds me of https://github.com/protocolbuffers/protobuf/issues/1491 , which has effectively been WONTFIX (why does github not have this useful distinction?) because Google are happy with how it works and it's really difficult to make this particular thing work with the (also broken) Python module import system.

squigz• 2 years ago

What distinction? Issues can have custom labels, and there is a default `wontfix` label

throwawayffffas2• 2 years ago

7764c864b and 0264866ce, right? I should be able to sync to those with git checkout and see which one dropped it, yeah? Well, I'll spare you the effort and just say that BOTH OF THEM have the old code in it.

When you make a merge commit, the merge commit contains all the changes. It's what happens when you fix a merge conflict. The fix for the conflict only exists in the merge commit. Similarly you can just add whatever you want in the merge, and it won't appear in any other commit.

throwawayffffas2• 2 years ago

That actually installed "protobuf-24.4,1" which is some insane version number I'd never seen before. All of my other systems are all running 3.x.x type versions.

They obviously changed their version numbering convention at some point, and this is protobuf 2.4.something.

Honestly, I see the shit she tripped on all the time. It doesn't even register anymore.

TheRealDunkirk• 2 years ago

Seems like precisely the sort of thing ESR's new de-autotools tool is designed to eliminate. https://gitlab.com/esr/autodafe

kidintech• 2 years ago

Piper and Protobufs lmao. Based on personal experience, the two services are best used as litmus tests of a person’s character — the most insufferable googlers I've met are fans of both. Any other usage of either generally results in undescribable frustration.

Hitting every branch on the way down

Related Stories