From: Elijah Newren <email@example.com> To: Junio C Hamano <firstname.lastname@example.org> Cc: Elijah Newren via GitGitGadget <email@example.com>, Git Mailing List <firstname.lastname@example.org> Subject: Re: [PATCH 2/4] t6038: fix test with obviously incorrect expectations Date: Mon, 3 Aug 2020 08:36:28 -0700 Message-ID: <CABPp-BHNebxZK1-tXx0HMmPDuRoj=_XWG=pVJ2HCvTkttvA4oQ@mail.gmail.com> (raw) In-Reply-To: <email@example.com> On Sun, Aug 2, 2020 at 12:57 PM Junio C Hamano <firstname.lastname@example.org> wrote: > > "Elijah Newren via GitGitGadget" <email@example.com> writes: > > > From: Elijah Newren <firstname.lastname@example.org> > > > > t6038.11, 'cherry-pick patch from after text=auto' was set up so that on > > a branch with no .gitattributes file, you cherry-picked a patch from a > > branch that had a .gitattributes file (containing '* text=auto'). > > Further, the two branches had a file which differed only in line > > endings. In this situation, correct behavior is not well defined: > > should the .gitattributes file affect the merge or not? > > I'd say that it is probably more intuitive to expect whatever > attributes set on the currently checked out and receiving the > cherry-picked change would take effect, but I do agree with you that > this is not well defined. Turns out this question is kind of important since merge-ort does not naturally get to take advantage of the existing infrastructure for attribute handling; I have to implement some stuff to trick it into using it, and the stuff I implement makes it glaringly obvious what choice is being made. Your answer here doesn't really help me since it's not even applicable in some cases. Anyway, I'll list a bunch of questions I'm facing with it, but if you want you can skip to the next quoted block of text and ignore this topic until I post the relevant patches for merge-ort, if you want. My questions about attribute handling in merges and which version(s) of .gitattributes should be used for three-way content merges: What if you don't have any version checked out, and are doing a merge in a bare-repo or are just redoing a merge (on some other branch) without touching the working directory or index just so you can view that other merge? In that case, your answer doesn't even apply and I need to implement something else. Also, what if you were doing a merge in a dirty working tree, where your .gitattributes was locally modified? Should the local .gitattributes file override the .gitattributes file recorded in history for how those versions are merged (which seems somewhat suggested by your answer)? Even if it makes the merge not reproducible by others? Are we okay with merging one way resulting in no conflicts while merges the other way (with the order of parents reversed) yielding conflicts due to use of different .gitattributes files? Also, there can be differences in what the user sees in a single merge while resolving, due to 'git checkout -m $CONFLICTED_PATH'. This happens in a few cases...explored in the next two paragraphs: What if the first parent had a .gitattributes file and the merge base did too (with same contents), but the second parent didn't? Do we use the .gitattributes from before the merge, despite the fact that the merge is going to delete the .gitattributes? If there are any conflicts and the users messes up a single file and needs to redo it, then a 'git checkout -m $CONFLICTED_PATH' later might give them a different result. Assuming there is no .gitattributes file in the first parent and none locally before merging, but the second parent did have a .gitattributes file, if we only pay attention to the .gitattributes file from the local working directory or the first parent, then we again run into a case where the merge may produce a different result than a subsequent 'git checkout -m $CONFLICTED_PATH'. What if the .gitattributes file itself needed to be merged? If it merges cleanly, should we use that clean merged version of .gitattributes to define the settings for all three-way content merges in the remainder of the merge? If it doesn't merge cleanly...should we just pick the one from the first parent? From the second? Throw a merge conflict stating that .gitattributes can't be merged and thus we don't know how to do content merging on any other conflicted path? (And what if the .gitattributes file only cleanly merges depending on if it's merged with one of the two .gitattributes settings from one of its parents?) > I think "changing attributes in the > middle may produce unexpected/undefined results---validate it when > you cross such a boundary" is a prudent advice we should give to the > users. Makes sense; especially given all the cases above. > Would it make more sense not to test ill defined behaviour at all > instead, I wonder? I'd be happy to toss the test and punt this conversation until I post the relevant patches for merge-ort, but we might not be kicking the can all that far down the road...
next prev parent reply index Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-08-02 6:33 [PATCH 0/4] Attr fixes Elijah Newren via GitGitGadget 2020-08-02 6:33 ` [PATCH 1/4] t6038: make tests fail for the right reason Elijah Newren via GitGitGadget 2020-08-02 18:17 ` Junio C Hamano 2020-08-02 19:10 ` Eric Sunshine 2020-08-03 15:06 ` Elijah Newren 2020-08-02 6:33 ` [PATCH 2/4] t6038: fix test with obviously incorrect expectations Elijah Newren via GitGitGadget 2020-08-02 19:57 ` Junio C Hamano 2020-08-03 15:36 ` Elijah Newren [this message] 2020-08-03 15:50 ` Junio C Hamano 2020-08-02 6:33 ` [PATCH 3/4] merge: make merge.renormalize work for all uses of merge machinery Elijah Newren via GitGitGadget 2020-08-02 19:23 ` Junio C Hamano 2020-08-03 15:07 ` Elijah Newren 2020-08-02 6:33 ` [PATCH 4/4] checkout: support renormalization with checkout -m <paths> Elijah Newren via GitGitGadget 2020-08-03 18:41 ` [PATCH v2 0/4] Attr fixes Elijah Newren via GitGitGadget 2020-08-03 18:41 ` [PATCH v2 1/4] t6038: make tests fail for the right reason Elijah Newren via GitGitGadget 2020-08-03 18:41 ` [PATCH v2 2/4] t6038: remove problematic test Elijah Newren via GitGitGadget 2020-08-03 18:41 ` [PATCH v2 3/4] merge: make merge.renormalize work for all uses of merge machinery Elijah Newren via GitGitGadget 2020-08-03 18:41 ` [PATCH v2 4/4] checkout: support renormalization with checkout -m <paths> Elijah Newren via GitGitGadget
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CABPp-BHNebxZK1-tXx0HMmPDuRoj=_XWG=pVJ2HCvTkttvA4oQ@mail.gmail.com' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Mailing List Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/git/0 git/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 git git/ https://lore.kernel.org/git \ email@example.com public-inbox-index git Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git