Re: [PATCH v10 00/36] Add directory rename detection to git
On Mon, Apr 23, 2018 at 4:46 PM, Junio C Hamanowrote: > Elijah Newren writes: > >> Out of 53288 merge commits with exactly two parents in linux.git: >> - 48491 merged identically >> - 4737 merged the same other than a few different "Auto-merging >> " output lines (as expected due to patch 35/36) >> - 53 merged the same other than different "Checking out files: ..." >> output (I just did a plain merge; no flags like --no-progress) >> - the remaining 7 commits had non-trivial merge differences, all >> attributable to directory rename detection kicking in >> >> So, it looks good to me. If anyone has suggestions for other testing >> to do, let me know. > > There must have been some merges that stopped due to conflicts among > those 50k, and I am interested to hear how they were different. Or > are they included in the above numbers (e.g. among 48491 there were > ones that stopped with conflicts, but the results these conflictted > merge left in the working tree and the index were identical)? They are included in the categories listed above. What my comparison did was for each of the 53288 commits: 1) Do the merge, capture stdout and stderr, and the exit status 2) Record output of 'git ls-files -s' 3) Record output of 'git status | grep -v detached' 4) Record contents of every untracked file (could be created e.g. due to D/F conflicts) 5) Record contents of 'git diff -M --staged' 6) Record contents of 'git diff -M' (all of this stuff in 1-6 is recorded into a single text file with some nice headers to split the sections up). 7) Repeat steps 1-6 with the new version of git, but recording into a different filename 8) Compare the two text files to see what was different between the two merges, if anything. (If they are different, save the files somewhere for me to look at later.) Then after each merge, there's a bunch of cleanup to make sure things are in a pristine state for the next merge.
Re: [PATCH v10 00/36] Add directory rename detection to git
Elijah Newrenwrites: > Out of 53288 merge commits with exactly two parents in linux.git: > - 48491 merged identically > - 4737 merged the same other than a few different "Auto-merging > " output lines (as expected due to patch 35/36) > - 53 merged the same other than different "Checking out files: ..." > output (I just did a plain merge; no flags like --no-progress) > - the remaining 7 commits had non-trivial merge differences, all > attributable to directory rename detection kicking in > > So, it looks good to me. If anyone has suggestions for other testing > to do, let me know. There must have been some merges that stopped due to conflicts among those 50k, and I am interested to hear how they were different. Or are they included in the above numbers (e.g. among 48491 there were ones that stopped with conflicts, but the results these conflictted merge left in the working tree and the index were identical)?
Re: [PATCH v10 00/36] Add directory rename detection to git
Hi Junio, On Thu, Apr 19, 2018 at 8:05 PM, Junio C Hamanowrote: >> On Thu, Apr 19, 2018 at 10:57 AM, Elijah Newren wrote: >>> This series is a reboot of the directory rename detection series that was >>> merged to master and then reverted due to the final patch having a buggy >>> can-skip-update check, as noted at >>> https://public-inbox.org/git/xmqqmuya43cs@gitster-ct.c.googlers.com/ >>> This series based on top of master. > > The series as-is is fine, I think, from the maintainer's point of > view. Thanks. Sorry to be a pest, but now I'm unsure how I should handle the next round. I've got: - two minor fixup commits that can be trivially squashed in (not yet sent), affecting just the final few patches - a "year" vs "years" typo in commit message of patch 32 (which is now in pu as commit 3daa9b3eb6dd) - an (independent-ish) unpack_trees fix (Message-ID: 20180421193736.12722-1-new...@gmail.com), possibly to be supplemented by another fix/improvement suggested by Duy Should I... - send out a reroll of everything, and include the unpack_trees fix(es) in the series? - just resend patches 32-36 with the fixes, and renumber the patches to include the unpack_trees stuff in the middle? - just send the two fixup commits, ignore the minor typo, and keep the unpack_trees fix(es) as a separate topic that we'll just want to advance first? - something else? Thanks, Elijah
Re: [PATCH v10 00/36] Add directory rename detection to git
On Thu, Apr 19, 2018 at 10:57 AM, Elijah Newrenwrote: > Additional testing: > > * I've re-merged all ~13k merge commits in git.git with both > git-2.17.0 and this version of git, comparing the results to each > other in detail. (Including stdout & stderr, as well as the output > of subsequent commands like `git status`, `git ls-files -s`, `git > diff -M`, `git diff -M --staged`). The only differences were in 23 > merges of either git-gui or gitk which involved directory renames > (e.g. git-2.17.0's merge would result in files like 'lib/tools.tcl' > or 'po/ru.po' instead of the expected 'git-gui/lib/tools.tcl' or > 'gitk-git/po/ru.po') > > * I'm trying to do the same with linux.git, but it looks like that will > take nearly a week to complete... Results after restarting[1] and throwing some big hardware at it to get faster completion: Out of 53288 merge commits with exactly two parents in linux.git: - 48491 merged identically - 4737 merged the same other than a few different "Auto-merging " output lines (as expected due to patch 35/36) - 53 merged the same other than different "Checking out files: ..." output (I just did a plain merge; no flags like --no-progress) - the remaining 7 commits had non-trivial merge differences, all attributable to directory rename detection kicking in So, it looks good to me. If anyone has suggestions for other testing to do, let me know. [1] Restarted so it could include my unpack_trees fix (from message-id20180421193736.12722-1-new...@gmail.com) plus a couple minor fixup commits (fixing some testcase nits and a comment typo).
Re: [PATCH v10 00/36] Add directory rename detection to git
Elijah Newrenwrites: > On Thu, Apr 19, 2018 at 10:57 AM, Elijah Newren wrote: >> This series is a reboot of the directory rename detection series that was >> merged to master and then reverted due to the final patch having a buggy >> can-skip-update check, as noted at >> https://public-inbox.org/git/xmqqmuya43cs@gitster-ct.c.googlers.com/ >> This series based on top of master. > > ...and merges cleanly to next but apparently has some minor conflicts > with both ds/lazy-load-trees and ps/test-chmtime-get from pu. > > What's the preferred way to resolve this? Rebase and resubmit my > series on pu, or something else? The series as-is is fine, I think, from the maintainer's point of view. Thanks.
Re: [PATCH v10 00/36] Add directory rename detection to git
On Thu, Apr 19, 2018 at 11:35 AM, Elijah Newrenwrote: > On Thu, Apr 19, 2018 at 10:57 AM, Elijah Newren wrote: >> This series is a reboot of the directory rename detection series that was >> merged to master and then reverted due to the final patch having a buggy >> can-skip-update check, as noted at >> https://public-inbox.org/git/xmqqmuya43cs@gitster-ct.c.googlers.com/ >> This series based on top of master. > > ...and merges cleanly to next but apparently has some minor conflicts > with both ds/lazy-load-trees and ps/test-chmtime-get from pu. > > What's the preferred way to resolve this? Rebase and resubmit my > series on pu, or something else? Sorry, user error; there are no conflicts with my series. (I accidentally included Junio's interim round of my own series and while trying to spot problems I saw commits from these other series touching relevant files in what looked like nearby areas. Directly merging with these other two series or even merging all of pu before en/rename-directory-detection-reboot followed by individually merging later series has no conflicts with any of my changes.)
Re: [PATCH v10 00/36] Add directory rename detection to git
On 4/19/2018 2:41 PM, Stefan Beller wrote: On Thu, Apr 19, 2018 at 11:35 AM, Elijah Newrenwrote: On Thu, Apr 19, 2018 at 10:57 AM, Elijah Newren wrote: This series is a reboot of the directory rename detection series that was merged to master and then reverted due to the final patch having a buggy can-skip-update check, as noted at https://public-inbox.org/git/xmqqmuya43cs@gitster-ct.c.googlers.com/ This series based on top of master. ...and merges cleanly to next but apparently has some minor conflicts with both ds/lazy-load-trees and ps/test-chmtime-get from pu. What's the preferred way to resolve this? Rebase and resubmit my series on pu, or something else? If you were to base it off of pu, this series would depend on all other series that pu contains. This is bad for the progress of this series. (If it were to be merged to next, all other series would automatically merge to next as well) If the conflicts are minor, then Junio resolves them; if you want to be nice, pick your merge point as git checkout origin/master git merge ds/lazy-load-trees git merge ps/test-chmtime-get git tag my-anchor and put the series on top of that anchor. If you do this, you'd want to be reasonably sure that those two series are not in too much flux. I believe ds/lazy-load-trees is queued for 'next'. I'm not surprised that there are some conflicts here. Any reference to the 'tree' member of a commit should be replaced with 'get_commit_tree(c)', or 'get_commit_tree_oid(c)' if you only care about the tree's object id. I think Stefan's suggestion is the best approach to get the right conflicts out of the way. Thanks, -Stolee
Re: [PATCH v10 00/36] Add directory rename detection to git
On Thu, Apr 19, 2018 at 11:35 AM, Elijah Newrenwrote: > On Thu, Apr 19, 2018 at 10:57 AM, Elijah Newren wrote: >> This series is a reboot of the directory rename detection series that was >> merged to master and then reverted due to the final patch having a buggy >> can-skip-update check, as noted at >> https://public-inbox.org/git/xmqqmuya43cs@gitster-ct.c.googlers.com/ >> This series based on top of master. > > ...and merges cleanly to next but apparently has some minor conflicts > with both ds/lazy-load-trees and ps/test-chmtime-get from pu. > > What's the preferred way to resolve this? Rebase and resubmit my > series on pu, or something else? If you were to base it off of pu, this series would depend on all other series that pu contains. This is bad for the progress of this series. (If it were to be merged to next, all other series would automatically merge to next as well) If the conflicts are minor, then Junio resolves them; if you want to be nice, pick your merge point as git checkout origin/master git merge ds/lazy-load-trees git merge ps/test-chmtime-get git tag my-anchor and put the series on top of that anchor. If you do this, you'd want to be reasonably sure that those two series are not in too much flux. Thanks, Stefan
Re: [PATCH v10 00/36] Add directory rename detection to git
On Thu, Apr 19, 2018 at 10:57 AM, Elijah Newrenwrote: > This series is a reboot of the directory rename detection series that was > merged to master and then reverted due to the final patch having a buggy > can-skip-update check, as noted at > https://public-inbox.org/git/xmqqmuya43cs@gitster-ct.c.googlers.com/ > This series based on top of master. ...and merges cleanly to next but apparently has some minor conflicts with both ds/lazy-load-trees and ps/test-chmtime-get from pu. What's the preferred way to resolve this? Rebase and resubmit my series on pu, or something else?