On 23-05-29 10:15AM, Stefan Sperling wrote:
> On Sun, May 28, 2023 at 08:33:20PM +0200, Omar Polo wrote:
> > On 2023/05/28 19:26:48 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > > On Sun, May 28, 2023 at 05:11:07PM +0200, Omar Polo wrote:
> > > > On 2023/05/28 12:59:39 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > > > Don't know, I'm not sure we should be using $PWD in lib/ to change the
> > > > behaviour, although removing $PWD is not nice.
> > > > 
> > > > Admittedly I haven't noticed the issue so far and won't consider it a
> > > > bad thing anyway, as it did what it is supposed to do.
> > > 
> > > Another point of concern is that the current behaviour of got rm
> > > contradicts the behaviour of /bin/rm.
> > > 
> > > /bin/rm does not allow the removal of "." or "..", and rm * will never
> > > result in the removal of "."
> > 
> > but even with this 'got rm' doesn't match rm.  It just adds a quirks
> > for the current working directory but otherwise keeps removing empty
> > ones, right?  I don't like quirks that much fwiw.
> 
> Yes, this is true. /bin/rm * will only remove files, not directories.
> 
> > rm and 'got rm' work on two different levels: one at the filesystem
> > level, where both files and directories are tracked, the other at the
> > "git view" of the filesystem, where only file exists and directories
> > are a byproduct.  I don't think there's much worth in trying to keep
> > these two tools behaving the same but if we want to, then 'got rm'
> > should keep directories around unless -R.
> > 
> > (do we need got rmdir too then? :P)
> 
> From my point of view this is a question of usabilty vs. design-purity.
> 
> It is true that these are different levels of concern but in practice
> the version-controlled and on-disk layers are never fully separated from
> the user's point of view. Users mostly work with the on-disk layer as
> usual, and expect version control to add some extra convenience.
> 
> When there is a contradiction in behaviour most people will intuitively
> expect on-disk layer semantics, as Mikhail did. Because why would the
> version control system try to make things more difficult than they were
> without version control? Version control is supposed to make software
> development easier, not harder, right? ;-)
> 
> On the other hand people who really understand how the version control
> system works will tend to differentiate between these two layers and
> accept resulting differences in behaviour.
> 
> We should not require users to have such expertise. Aligning the
> version control system's behaviour with expectations of the regular
> "on-disk" user has benefits for usability but will introduce weird
> edge-cases and exceptions into the version control system.
> 
> It's a trade-off and a fine line to walk, and there is not always an
> obviously correct answer.
> 
> > On the other hand we could add an option, say -P to wink at cvs, to
> > avoid pruning empty directories, but I don't like the idea either to
> > be fair.  Noone will use it and we'll only have more code to maintain
> > and more words in the manpage.
> 
> Yeah, I'd rather not add an option because people will usually forget
> to pass it when it would be needed. In cases like this adding an option
> is just an easy way out for developers to say "here you go, I have solved
> this problem for you" without actually improving the workflow.
> 
> > I feel like the current behaviour is the one that sucks less.
> 
> I am happy to keep it as-is then. We can revisit this at any point
> if the issue ever gets raised again. I expect that people who work
> on the ports tree might run into this again eventually.

tbh, I think stsp's proposed change is a notable improvement over the
current behaviour--which is surprising imo. Purely from a POLA
perspective, I think we should go ahead with it. The added consistency
with both the filesystem layer and other VCSs (Fossil also leaves the
directory), and I think it is a definite improvement. (That's not to say
we should do things just to be the same as others, but that, in this
case, they have the right idea.) And then when you consider the target
audience, as stsp noted, this will invariably pop up again.

Full disclosure: I have not had time to review or test the diff, but I
think the change in behaviour is the right one.

btw, that's not to dismiss op's concerns either--they are valid and I
appreciate them. I weight the usability benefit a little more in this
case.

-- 
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68