On Fri, Jun 14, 2019 at 12:00:55PM +0200, SZEDER Gábor wrote:
> Update 'compat/obstack.{c,h}' from upstream, because they already use
> 'size_t' instead of 'long' in places that might eventually end up as
> an argument to malloc(), which might solve build errors with GCC 8 on
> Windows.
>
> The first patch just imports from upstream and doesn't modify anything
> at all, and, consequently, it can't be compiled because of a screenful
> or two of errors. This is bad for future bisects, of course.
>
> OTOH, adding all the necessary build fixes right away makes review
> harder...
One thing about your approach that makes it hard to review is that the
first commit obliterates all of our local changes, and then you have to
re-apply them individually. Looking at "git log" there aren't very many
in this case, so it's pretty easy to be sure you got them all (in some
cases this can be particularly nasty if the changes were themselves part
of conflict resolution, and so you have to pick them out of a merge).
I think a flow that better matches "what really happened" is to do more
of a vendor-branch approach: have a line of history that represents the
upstream changes (each one obliterating the last), and then periodically
merge that into our fork.
That can even retain bisectability as long as the commits along the
vendor branch don't actually try to build the code. Unfortunately our
initial import does try to build, so I think it already wrecks
bisectability, but we can undo that now. So e.g.,:
# start at e831171d67 (Add obstack.[ch] from EGLIBC 2.10, 2011-08-21)
git checkout -b upstream-obstack e831171d67
# undo build changes to restore bisection; ideally this would have
# been done back then, but it's too late now
sed -i /obstack/d Makefile
git commit -am 'strip out obstack building'
# but of course in our merged version we want that back, so let's
# do a noop merge to represent that.
git checkout master ;# or whatever feature branch you're working on
git merge -s ours upstream-obstack
# and now with a sane vendor branch established, we can proceed to do
# a real update there
git checkout upstream-obstack
cp /path/to/obstack.[ch] compat/
git commit -am 'update obstack'
# and now we are free to merge that in, getting a real 3-way merge
# between our changes and what happened upstream.
git checkout master
git merge upstream-obstack
Now, if you try this you may find that the conflicts are pretty horrid.
And the result may end up way less readable than your cherry-picks (and
harder to resolve in the first place). I claim only that:
1. This represents in the history graph more clearly the actual
sequence of events.
2. Its saves you from digging up the set of changes that have been
applied since our last upstream import.
So in this case the way you did it may well be the best way. But I offer
it as an alternative. :)
-Peff
