Re: [lmi] [PATCH] Use submodules for, and newer versions of, libxml2 and libxslt

[Vadim Zeitlin]

On Thu, 1 Oct 2020 15:06:04 +0000 Greg Chicares <gchicares@sbcglobal.net> wrote:

GC> Reading this in full on a less hectic day, I'll mention that I had already
GC> merged the branch, but not pushed it. However, adapting to your forced
GC> changes is either
GC>  - so easy because of my git-fu that I could do it in my sleep, even
GC>    with a postoperative infection, or
GC>  - so hard because of my lack of git-fu that I must practice until it
GC>    becomes easy.
GC> I won't say which of those is more true, but I'll say what I did.

 I think it's even easier what you did, although you did something that was
both correct and easy enough.

GC> # Fetch rebased remote
GC> 
GC> $git fetch xanadu
GC> remote: Enumerating objects: 10, done.
GC> remote: Counting objects: 100% (10/10), done.
GC> remote: Compressing objects: 100% (4/4), done.
GC> remote: Total 10 (delta 4), reused 10 (delta 4), pack-reused 0
GC> Unpacking objects: 100% (10/10), done.
GC> From https://github.com/vadz/lmi
GC>  + 5395409f...d217f203 submodule-for-xml-libs -> 
xanadu/submodule-for-xml-libs  (forced update)

 BTW, this "forced update" is rather unobtrusive but is something to look
for carefully. Of course, I hope that it won't happen often, i.e. that in
the future we'll finish with testing all the changes first before creating
the PRs, and I'll definitely always warn you before hand if it does happen
again, but if I somehow don't, please pay special attention to this part of
the message from git-fetch.

GC> # Move files affected by unpushed merge aside (just because I want to)
GC> # It doesn't matter that one had been deleted
GC> 
GC> $mkdir /opt/lmi/stash/submod
GC> $mv -v $(git diff --name-only origin/master) /opt/lmi/stash/submod
...
GC> git ls-files --deleted | xargs -t git checkout --
...
GC> # Drop unpushed merge, interactively changing each commit to "drop"
GC> # This is like 'git reset [some-argument]', but I can see what I'm doing
GC> 
GC> $git rebase --interactive

 Judging from the name of the temporary directory, I'd say that you already
know about git-stash command, but if you do, why don't you use it? Doing

        $ git reset origin/master
        $ git stash push --include-untracked -m 'Save old merge results'

would do exactly the same thing, except you'd have a useful name for the
stash and, of course, it's also shorter, faster and less error-prone.

 I do see the comment about "see what I'm doing" above, but I am not really
sure I understand it, i.e. what is the trouble with git-reset?

GC> # Repeat the merge
GC> 
GC> git merge xanadu/submodule-for-xml-libs

 As an aside, this happened to be a fast-forward merge, but if it were not,
this would have created a merge commit -- which, IMHO, would be a perfectly
cromulent thing to do (in fact so much so, that I use --no-ff with all my
git-merge commands to ensure that there are always merge commits, even if
fast forwarding would be possible). But you've expressed your dislike of
merges, and the resulting non-linear history, before and if you still have
this sentiment, you would have to rather do

        $ git cherry-pick ..xanadu/submodule-for-xml-libs

to replay all the branch commits on master (this is the same thing that
creating a local branch based on that one, rebasing it on master and then
fast-forward-merging it would do). Again, IMO this is just some extra work
in pursuit of a very dubious goal, so I wouldn't do it like this and would
just run the same git-merge command you did, but I mention it just in case
you really, really want to avoid merges.

GC> # Take a look

 FWIW it looks perfect, thanks again for merging this so quickly!

 I plan to make more PRs to switch to using submodules for wx and wxPdfDoc
too in the near future. I don't know if it's worth doing it for all
libraries from install_miscellania.make, but I'd like to do it for at least
xmlwrapp (which, BTW, could/ought to be installed from install_x*.sh now).
Please let me know if you have any objections.

 Thanks in advance,
VZ

pgpe3ByXq55ex.pgp
Description: PGP signature