Mark Phippard wrote on Fri, Feb 25, 2022 at 10:39:13 -0500:
> On Fri, Feb 25, 2022 at 10:19 AM Daniel Shahaf <d...@daniel.shahaf.name> 
> wrote:
> >
> > Nathan Hartman wrote on Thu, 24 Feb 2022 23:44 +00:00:
> > > On Thu, Feb 24, 2022 at 2:33 PM Mark Phippard <markp...@gmail.com> wrote:
> > >>
> > >> On Thu, Feb 24, 2022 at 10:51 AM Lorenz <loren...@yahoo.com> wrote:
> > >>
> > >> > just discovered, that merging into a RO file succeeds.
> > >> > I my case the file is RO because of a needs-lock property.
> > >> > So when I try to commit I can't.
> > >> >
> > >> > This effect ist at least annoying.
> > >> > There is a reason why the file is read only.
> > >>
> > >> It is interesting both that it was able to do this and also that this
> > >> has never come up before. I am not sure I can wrap my head around what
> > >> I think should even happen here. There is no way you could be expected
> > >> to obtain locks on these files before merging.
> >
> > Why can't we expect that?
> 
> I had not considered your proposal to just lock everything before the
> merge. I agree that would be a solution. I cannot predict user
> reaction because it is hard to know what kind of scale we are talking
> about.

To be clear, I didn't intend that as a "solution"; only as a "first
approximation workaround", as I said there.

> When I said we could "not expect" what I was getting at is that it
> would require the user to figure out what files the merge was going to
> modify BEFORE they do the merge and lock those files. That does not
> seem reasonable ... again I did not consider the simpler lock
> everything idea.

Why wouldn't that be reasonable?  When merging a feature branch, it's
fairly easy to find "all files the branch touched" — in the UI, «svn
diff --summarize» in the UI; in the FS backend, walking over the
changed-paths information without having to parse dir reps — and,
barring copies/renames on the merged-to branch, we wouldn't ever have
to lock a file the branch didn't touch, would we?

I don't know off the top of my head how we handle copies/renames in the
merge target.

> > >> I guess ideally merge would obtain locks but I just do not know what
> > >> merge should do if it cannot obtain the lock. I guess it should
> > >> behave the same way it would if the file was not present due to it
> > >> being a sparse working copy.
> > >
> > > Hopefully there would be a visible warning if that happened!
> > >
> > >> Maybe merge should just refuse to run at all if it detects any
> > >> svn:needs-lock properties in the WC?
> > >
> > > Then it would never be able to run; I think you meant: refuse to run
> > > if we don't hold the lock on a file that has the svn:needs-lock
> > > property. However, it would be rather irritating if the refusal to run
> > > were caused by files unaffected by the merge.
> 
> I am projecting a bit here based on memory of now 10+ year old discussions.
> 
> At the beginning of merge, the process has no idea what it is going to
> merge so it has no way to check for locks.
> 
> I am assuming it is difficult for the merge process at the point of
> merging a specific file to trigger a lock workflow before it does it.
> The merge process kind of has to do all of this before it begins and
> before it even knows what it is going to do. Those assumptions could
> be wrong, or someone could rewrite the whole thing etc. I was just
> being realistic that this is a problem that we are probably past the
> point of solving.
> 
> > Or if the user deliberately did the merge without intending to commit it.
> >
> > How about making this situation a conflict?  "local file not locked,
> > incoming edit upon merge".  Then the file is not silently changed, but
> > the user can use --accept or «svn resolve» to make the changes anyway if
> > they know that's what they want.
> 
> That seems like it could be a viable option given that we have
> improved our process to include these sort of things with all of the
> tree conflict work we have done and the additional workflows we added
> for it. This definitely seems like the best idea for someone to
> pursue.

I wonder if it's possible to prototype this with svnraisetreeconflict(1)
(from tools/).  There's no existing enum value for "not locked", but
"incoming edit" and "upon merge" both already exist in the API.

Cheers,

Daniel


> Your lock everything idea before merging would be a good workaround if
> we were to just make merge start to fail if it runs into a lock rather
> than what it is doing today.
> 
> Mark

Reply via email to