When I did the earlier pick ups of Henry's work I didn't realize merge was the way to go and instead did it the hard way.
I have been using git with advice from random google searches and as always that is a very dangerous way to proceed. I guess I have to take some time and actually rtfm. We will slowly get better. On Fri, May 27, 2016 at 4:05 PM, <neit...@gaertner.de> wrote: > [This one contains no criticism, just explanations. With, again, > views from the command line world, and some problem resolution > at the end.] > > > Well done, Eric! > > I am referring to the "git merge"s you recently did, the first one > from last Sunday being: > > commit 4d69436d09a9a10544ca5b8cbf6280fc442ad7b6 > Merge: 1b098dd 1a1008f > Author: Eric Iverson <eiver...@jsoftware.com> > Date: Sun May 22 13:38:48 2016 -0400 > > bpctlz > > Followed up by the further four branch merges on Thursday. > > This much better than your > > commit 2135cfd8421c656afc961f7add4b2f0d7f9030d1 > Author: Eric Iverson <eiver...@jsoftware.com> > Date: Wed May 11 16:39:03 2016 -0400 > > parser > > where you integrated Henry's work by applying a large patch. > > The downside of this patch is that it lumps the original, individual > commits together. (This is sometime wanted, but often not.) You > also lost all of Henry's original commit messages which had been: > > Small instruction tweaks; fix error-message omitted when referring > to undefined x. etc. in explicit def > > Add testcases and final tweaks to commentary > > Reorder name-stacking to reduce testing and function-call time. > Renumber parser lines so that token-error info is in the line > number. Use locals to reduce pointer dereferencing. Total perf > improvement now 9.2% > > Rewrite nvr routines for speed. Don't call nvrredef on unchanged > names. Much commentary. This brings the new parser's total > lint-on-lint improvement to 9.1% > > Parser rewritten, with 6.7% speedup on lint-on-lint. The only > interface change is that the caller is no longer required to > allocate and initialize the parser stack. > > and substituted them all by that single word "parser". > > Merges (as you now did) preserve all this info while causing you > less work. Another huge advantage: Change dates and authorship > are preserved -- "git blame" will now associate buggy lines in > that merged code with Henry and not with you. > > Because you have been doing merges like a pro, you certainly > need no further convincing. I just want to confirm that you > are doing things perfectly now. > > Unless a merge involves significant integration work, it is > customary to just stick with the default merge commit message. > You already did that once before: > > commit 33d0816a15117cb3e0af04b13a02d7e8b87a916f > Merge: 6c0777c 7a540ba > Author: Eric Iverson <eiver...@jsoftware.com> > Date: Tue Jan 19 11:14:20 2016 -0500 > > Merge branch 'master' of ssh://www.jsoftware.com:49198/jsource > > > There is one small problem remaing now, though: > > Because you applied Henry's work from the "parser" branch by applying > it as a patch (instead of a merge), git isn't aware that these changes > are already present in the trunk. > > Visually, one can see this in the github "network" view: > > <https://gaertner.de/~neitzel/eref/parse-network.png> > > or, equivalently, the git command which shows unmerged changes which > reside on topic branches only: > > > jsource 581 > git show-branch -a --topic | cut -c -72 > * [master] simlify version template > ! [origin/0quotecobignum] Leading zeros caused large numbers not to be > ! [origin/Edotnonchar] Ensure adjusting the lower interval bound doesn > ! [origin/HEAD] simlify version template > ! [origin/bpctlz] Perf improvement: fix bsum() to use word-wide oper > ! [origin/inplacealias] Added commentary. It is OK to overwrite th > ! [origin/makevsdoc] First try at modifying jsoftware repository > ! [origin/master] simlify version template > ! [origin/oldbugs] Fix uninitialized variable encountered during > ! [origin/parser] Small instruction tweaks; fix error-message o > ! [origin/shapeandfill] Add more tests > ! [origin/tallstackcrash] Fix so exec on fill-cell in " does > ------------ > + [origin/inplacealias] Added commentary. It is OK to overwr > + [origin/inplacealias^] Fix local-to-global aliasing during > + [origin/oldbugs] Fix uninitialized variable encountered dur > + [origin/oldbugs^] Back out change to prevent ^:gerund from > + [origin/oldbugs~2] Fix bugs in explicit definition: > + [origin/oldbugs~3] Fix old bugs: > + [origin/parser] Small instruction tweaks; fix error-message > + [origin/parser^] Add testcases and final tweaks to commenta > + [origin/parser~2] Reorder name-stacking to reduce testing a > + [origin/parser~3] Rewrite nvr routines for speed. Don't ca > + [origin/parser~4] Parser rewritten, with 6.7% speedup on li > *+++++++++++ [origin/makevsdoc^] debug start command > jsource 582 > > > The same thing with funky colors, and not eliding commits merged to > the master branch: > > <https://gaertner.de/~neitzel/eref/parse-show-branch.png> > > > Now, how to resolve this? > > First, we should check check manually that Eric applied all code from > Henry's the "parser" branch thruthfully. We can do this by recreating > the respective diffs and comparing them: > > # First, Ericss single commit, diff against it's parent (= prior version). > # The commit id 2135cfd is taken from waaay earlier in this message: > > jsource 605 > git diff 2135cfd^..2135cfd > /tmp/erics-single-diff > > # find the place where Henry created his branch, > # and create the diff encompassing the entire branch: > > jsource 606 > git merge-base origin/parser origin/master > 725c4dcbd4d3cc01754d73aa27a2f6a68b70dda6 > jsource 607 > git diff 725c4dcbd..origin/parser > /tmp/henry-branch-diff > > # compare the two via checksumming: > > jsource 608 > md5 /tmp/*-diff > MD5 (/tmp/erics-single-diff) = 5c14ece15d47c1652dd73a9982b20b4c > MD5 (/tmp/henry-branch-diff) = 5c14ece15d47c1652dd73a9982b20b4c > > Verdict: it's the same. > > > At this point, Eric might be tempted to > > 1. undo the single patch ("git revert 2135cfd") > 2. git merge parser > > This would technically work but this form of "rewritig history" (the > entire history since Wed May 11) is not good for a public branch as > exposed as the master branch. One would need *everybody* to update > their git copies with the new history or a heap of confusion ensues > ("what exactly was in v2.c on May, 19th?"). One does that kind of thing > (fixing history) on "mostly private" branches only. On publicly > branches, this is a big no-no and frowned upon. Within our current, > small J src community, it would be viable. You'd to give a big > shout "please everybody pull!" though, to avoid said confusion. > > > Luckily for Eric, things can be done much simpler: it is still possible > to do a "git merge" of the "parser" branch, just by doing the > > git merge parser > > At this point, the parser source code is already on the branch and nothing > in the code will be changed. It will also still be attributed to Eric, > not Henry: > > bjsource 629 > git blame jsrc/p.c | grep token-in-error > 2135cfd8 (Eric Iverson 2016-05-11 16:39:03 -0400 208) if(i&6){ // if > not PNOMATCH... (we test this way to start calculating the token-in-error > offset) > 2135cfd8 (Eric Iverson 2016-05-11 16:39:03 -0400 213) *dci = > (I)stack[(i&6)|1]; // set the token-in-error, using the offset encoded > into i > > > BUT Henry's commits 0b4e216..c18f66a will now be be seen as integrated > into the master branch (I did it on my local master, of course): > > jsource 632 > git log --oneline --graph | cut -c -55 > * 71deab3 Merge remote-tracking branch 'origin/parser > |\ > | * c18f66a Small instruction tweaks; fix error-message > | * 8b39f2f Add testcases and final tweaks to commentar > | * 386b7d6 Reorder name-stacking to reduce testing and > | * 9fe7c8a Rewrite nvr routines for speed. Don't call > | * 0b4e216 Parser rewritten, with 6.7% speedup on lint > * | edb3ec2 simlify version template > * | 19dee12 Henry - tallstackcrash > |\ \ > | * | 696512e Fix so exec on fill-cell in " does not RE > | * | 4443998 Fix buffer overrun and ASSERT inside FPDE > * | | 86baf9c Henry - 0quotecobignum > |\ \ \ > | * | | fcbb420 Leading zeros caused large numbers not > | * | | 2a4cb96 Make 0 ". bigvalue work even when bigva > | |/ / > [ ^ this line going WAY back, branching off from the Mon Apr 11 version.] > > > 'Nuff for today, > Martin > ---------------------------------------------------------------------- > For information about J forums see http://www.jsoftware.com/forums.htm ---------------------------------------------------------------------- For information about J forums see http://www.jsoftware.com/forums.htm
