darcs

Issue 1177 Folders cause unexpected dependencies...

Title Folders cause unexpected dependencies...
Priority wishlist Status wont-fix
Milestone Resolved in
Superseder forcible commutation maintaining patch identity
View: 1559
Nosy List btcoburn, darcs-devel, dmitry.kurochkin, ganesh, kowey, thorkilnaur, twb
Assigned To
Topics

Created on 2008-10-27.08:25:06 by btcoburn, last changed 2009-08-27.14:31:18 by admin.

Files
File name Uploaded Type Edit Remove
accept-issue1177_-adddir-patches-result-in-silly-dependencies_.dpatch twb, 2009-01-28.00:47:57 text/x-darcs-patch
folder_dependency_example.bash btcoburn, 2008-10-27.08:25:04 application/octet-stream
unnamed twb, 2009-01-28.00:47:57 text/plain
Messages
msg6477 (view) Author: btcoburn Date: 2008-10-27.08:25:04
Recording folder addition/deletion explicitly causes unexpected and unnecessary
dependencies. I think it would help a lot to have an options to:
- NEVER record folder creation (file creation already caries this information)
- AUTO-CREATE folders as needed when changing files in the working directory
- DELETE folders when they become empty (i.e. by obliterating the patch that
auto-created the folder)

While I can imagine ways to work around these dependencies, I should not have to
fight with darcs on this! I've attached a script demonstrating this kind of
unnecessary dependency.
Attachments
msg7203 (view) Author: twb Date: 2009-01-26.06:58:10
On Sun, Oct 26, 2008 at 09:25:04PM +0000, Ben Coburn wrote:
> Recording folder addition/deletion explicitly causes unexpected and unnecessary
> dependencies. I think it would help a lot to have an options to:
> - NEVER record folder creation (file creation already caries this information)
> - AUTO-CREATE folders as needed when changing files in the working directory
> - DELETE folders when they become empty (i.e. by obliterating the patch that
> auto-created the folder)

Mercurial (hg) behaves this way, and one consequence is that it is
impossible to have an empty directory recorded in the repository.
msg7216 (view) Author: btcoburn Date: 2009-01-27.00:01:31
Having thought more deeply about this in the mean time, I need to amend 
this recommendation. The main issue is that folders create dependencies
between patches. Auto-creation of folders will allow us to discard this
unnecessary dependency while retaining forward and backward
compatibility. I think the best behavior is to:

- record folders as normal (for backward compatibility) [1]

- AUTO-CREATE folders as needed when changing files in the working 
directory

- DO NOT SEARCH FOR patches that create missing folders when resolving 
patch dependencies (because we auto-create folders if missing) [2]

- create folders as normal when found in patches (for flexibility to 
create empty folders) [1]


[1] darcs currently does this
[2] ignoring this class of arbitrary dependencies might give some nice 
side effects, like simpler/faster patch commutation on larger
repositories...
msg7218 (view) Author: kowey Date: 2009-01-27.11:14:36
Just to confirm:

On Tue, Jan 27, 2009 at 00:01:38 -0000, Ben Coburn wrote:
> - AUTO-CREATE folders as needed when changing files in the working 
> directory
> 
> - DO NOT SEARCH FOR patches that create missing folders when resolving 
> patch dependencies (because we auto-create folders if missing) [2]

Are you thinking about this in terms of a "deep" change in darcs, as
opposed to a user interface change?

I gather you're suggesting that it should be possible to create a patch
addfile "a/b" without that patch being preceeded by an "adddir a".

Or were you hoping for something much more on the surface?

Thanks!
msg7222 (view) Author: twb Date: 2009-01-27.12:28:49
On Tue, Jan 27, 2009 at 11:14:39AM -0000, Eric Kow wrote:
> 
> Eric Kow <kowey@darcs.net> added the comment:
> 
> Just to confirm:
> 
> On Tue, Jan 27, 2009 at 00:01:38 -0000, Ben Coburn wrote:
> > - AUTO-CREATE folders as needed when changing files in the working 
> > directory
> > 
> > - DO NOT SEARCH FOR patches that create missing folders when resolving 
> > patch dependencies (because we auto-create folders if missing) [2]
> 
> Are you thinking about this in terms of a "deep" change in darcs, as
> opposed to a user interface change?
> 
> I gather you're suggesting that it should be possible to create a patch
> addfile "a/b" without that patch being preceeded by an "adddir a".

Right; in hg, there are not "adddir" patches; directories are
implicitly created prior to applying a patch that adds a file, and
implicitly destroyed after the last version-controlled file in them is
removed (as long as there are no unrecorded changes still there, of
course).

I see this as a significant change for Darcs 3.0, but it might be
possible to keep backwards-compatibility and introduce it earlier with
some trickiness.

As a concrete example of where this bug is bloody annoying: consider
trying to extract the history of .bin/ru from the repo

    http://twb.ath.cx/Preferences

Which is non-trivial because addfile .bin/ru depends on adddir .bin,
which itself depends on the initial import patch and a whole swag of
other patches.

I successfully did extract the patches to

    http://code.haskell.org/~twb/ru
    http://twb.ath.cx/ru-0.1.1        # mirror

but if you get that second repo and then try to pull --match 'touch
.bin/ru' from the first repo, you'll see that Darcs completely falls
down and dies.  This means that any time I want to pull new ru changes
into the separate ru-only repo, I actually have to completely recreate
that repo from scratch (using the technique described in the first
patch in ru-0.1.1).
msg7224 (view) Author: btcoburn Date: 2009-01-27.16:09:51
On Jan 27, 2009, at 03:14, Eric Kow wrote:

>
> Eric Kow <kowey@darcs.net> added the comment:
>
> Just to confirm:

You are clipping out the first part, which makes a big difference in
backward compatibility. (Added back for reference.)

>
> On Tue, Jan 27, 2009 at 00:01:38 -0000, Ben Coburn wrote:
>> - record folders as normal (for backward compatibility) [1]
>>
>> - AUTO-CREATE folders as needed when changing files in the working
>> directory
>>
>> - DO NOT SEARCH FOR patches that create missing folders when
>> resolving patch dependencies (because we auto-create folders if
>> missing) [2]
>
> Are you thinking about this in terms of a "deep" change in darcs, as
> opposed to a user interface change?
>

This will be a "deep" change, however it should have a very "small"
effect on repository compatibility if we CONTINUE TO RECORD FOLDERS.
The only requirement is that we sever the dependencies caused by
folder creation that are currently causing this issue.

> I gather you're suggesting that it should be possible to create a
> patch addfile "a/b" without that patch being preceeded by an
> "adddir a".
>

Sort of, see the point I re-quoted.

A repository in this state must be VALID. However, this state should
only happen when you silently cherry-pick your way around a folder
creation patch (or pull from a repository that did). If you pull in
the whole repository, you should get a repository with all folders
properly created (that is compatible with current darcs).

Speculation: It should even be possible for users of current darcs to
"heal" a repository that has missed a folder through cherry-picking.
Just create a new atomic patch for the folder creation. It will
commute freely to the right place, and make current darcs happy.


Regards, Ben Coburn
msg7225 (view) Author: kowey Date: 2009-01-27.16:42:18
On Tue, Jan 27, 2009 at 16:09:55 -0000, Ben Coburn wrote:
> You are clipping out the first part, which makes a big difference in
> backward compatibility. (Added back for reference.)

Thanks for pointing that out.

I understand that by default you expect for darcs to continue to record
folders, and that you want to make it possible for a create "a/b" to
exist regardless of whether the corresponding create directory patch
exists.

Let me try again.  Is this modification you propose that:

  adddir d and addfile d/f should commute

I guess the result would be

  addfile d/f and adddir d

> This will be a "deep" change, however it should have a very "small"
> effect on repository compatibility if we CONTINUE TO RECORD FOLDERS.
> The only requirement is that we sever the dependencies caused by
> folder creation that are currently causing this issue.

My nervousness (still!) is that changes on a deep level however
innocuous they may seem can have rather wide repercussions.

It doesn't mean we should avoid them, just that we should make sure
we've given this a lot of thought first.  I suspect this is really
a darcs 3 kind of change rather than a darcs 2.x, for example, i.e.
something to be taken fairly seriously.

When we expect something to conflict, and it no longer does so, things
could go wrong

This is where my understand of patch theory and the implications thereof
fail me.   What are the backward compatibility consequences of this
small change?  In what way would darcs be relying on the fact that
addfile "d/f" does not commute with adddir "d"?

Can somebody create an example of how this change in semantics can
create an inconsistency with an older repository?

One practical UI consequence of not allowing them to commute, for
example, is that if we unpull the adddir d patch, we know that we must
also unpull the addfile d/f.  If we do allow them to commute, then we
lose this knowledge (at least via patch theory)

> A repository in this state must be VALID. However, this state should
> only happen when you silently cherry-pick your way around a folder
> creation patch (or pull from a repository that did). If you pull in
> the whole repository, you should get a repository with all folders
> properly created (that is compatible with current darcs).

Maybe Ian has something to say about this, or would be interested for
his work on camp.

PS. Thanks for your patience!  Let me know if I have still
missed something or need to be reading more closely.
msg7227 (view) Author: ganesh Date: 2009-01-27.23:26:38
This seems to me to be a specific instance of a general problem, namely that
having multiple "primitive" patches in the same "recorded" patch creates
spurious dependencies. I think it'd be better to deal with that problem than to
implement a hack like this, that makes it impossible to have empty directories.

As kowey points out, it would require changing the behaviour of commutation.
This is not something that we can do without a repo format switch and causing
incompatibility between versions of darcs. In darcs, merges are always
repeatable, and this proposal would change the behaviour of merge(rmdir foo,
mkfile foo/bar) from being a conflict into being ok.
msg7228 (view) Author: ganesh Date: 2009-01-27.23:27:57
BTW, as a practical tip if this issue is bothering you, never record
adddir/rmdir patches along with any other changes. That should basically
eliminate any dependencies that cause problems in future, though of course it
doesn't help with existing repos.
msg7229 (view) Author: twb Date: 2009-01-28.00:06:18
On Tue, Jan 27, 2009 at 11:26:41PM -0000, Ganesh Sittampalam wrote:
> This seems to me to be a specific instance of a general problem,
> namely that having multiple "primitive" patches in the same
> "recorded" patch creates spurious dependencies.

IMO the real problem is that "darcs rec -la d/f" implicitly records
"adddir d", and that it's not intuitive (even to me) that this will
break cherry-picking in the future.  And by the time you discover the
dependency, it's far too late.

Whereas if I "darcs record f g", I *expect* that to tie f and g together.

> I think it'd be better to deal with that problem than to implement a
> hack like this, that makes it impossible to have empty directories.

Do you have any thoughts on how to go about this?  Perhaps transplant
(issue938) is what you're hinting at?

As for empty directories: how useful are they?  As with chmod and
chown, it's not hard for a post-hook or makefile script to do a "mkdir
empty.d".

> As kowey points out, it would require changing the behaviour of
> commutation.  This is not something that we can do without a repo
> format switch and causing incompatibility between versions of darcs.

Granted.
msg7230 (view) Author: ganesh Date: 2009-01-28.00:13:30
> > I think it'd be better to deal with that problem than to implement a
> > hack like this, that makes it impossible to have empty directories.

> Do you have any thoughts on how to go about this?

This isn't anything to do with transplant, or at least it's only peripherally
related. I'd like it to be much easier to break up a patch into pieces when so
that one dependency doesn't block an entire commute. From memory, I think camp
handles this better than darcs.
msg7231 (view) Author: ganesh Date: 2009-01-28.00:15:27
> As for empty directories: how useful are they?

Probably not very. But disallowing them would create all sorts of special cases
that I think would be very ugly. An analogy would be the existence of 0; it's
not much direct use for counting things, but I can't really imagine a decent
number system without it.
msg7232 (view) Author: twb Date: 2009-01-28.00:31:39
On Tue, Jan 27, 2009 at 11:28:00PM -0000, Ganesh Sittampalam wrote:
> BTW, as a practical tip if this issue is bothering you, never record
> adddir/rmdir patches along with any other changes. That should
> basically eliminate any dependencies that cause problems in future,
> though of course it doesn't help with existing repos.

Yes, but it's difficult to remember to do so and wearying to see a
bunch of separate "Added a directory: foo." patches in the change log.
msg7233 (view) Author: btcoburn Date: 2009-01-28.00:34:01
On Jan 27, 2009, at 08:42, Eric Kow wrote:

>> This will be a "deep" change, however it should have a very "small"
>> effect on repository compatibility if we CONTINUE TO RECORD FOLDERS.
>> The only requirement is that we sever the dependencies caused by
>> folder creation that are currently causing this issue.
>
> My nervousness (still!) is that changes on a deep level however
> innocuous they may seem can have rather wide repercussions.
>
> It doesn't mean we should avoid them, just that we should make sure
> we've given this a lot of thought first.
>

I completely agree. Unfortunately, my mental model of darcs is not  
precise enough to give answers to all these details -- it's mostly  
based on reading the list for the last year or two and how darcs  
behaves in practice.

Regards, Ben Coburn
msg7234 (view) Author: twb Date: 2009-01-28.00:47:57
Attached is a patch against bugs/ that demonstrates the problem:
accidentally recording an adddir d along with an initial file d/f that
doesn't have any special significance other than the user happened to
record it first.  Other files in d/ then become impossible to
cherry-pick without also picking d/f. 

Mon Jan 26 18:21:43 EST 2009  Trent W. Buck <trentbuck@gmail.com>
  * Accept issue1177: adddir patches result in silly dependencies.
Attachments
msg7235 (view) Author: btcoburn Date: 2009-01-28.00:55:16
Sorry, you are still confusing me.

On Jan 27, 2009, at 08:42, Eric Kow wrote:

>
> Eric Kow <kowey@darcs.net> added the comment:
>
> On Tue, Jan 27, 2009 at 16:09:55 -0000, Ben Coburn wrote:
>> You are clipping out the first part, which makes a big difference in
>> backward compatibility. (Added back for reference.)
>
> Thanks for pointing that out.
>
> I understand that by default you expect for darcs to continue to  
> record
> folders, and that you want to make it possible for a create "a/b" to
> exist regardless of whether the corresponding create directory patch
> exists.
>

Up to here it makes sence.

> Let me try again.  Is this modification you propose that:
>
>  adddir d and addfile d/f should commute
>
> I guess the result would be
>
>  addfile d/f and adddir d
>

I do not think that I am proposing this, or that what I have described  
would require "adddir d" to commute with "addfile d/f".

In my model of darcs I think of commutation and resolving dependencies  
as different things (that is sorting patches, and making sure the  
right patches exist in a repo). Is this the wrong way to conceptualize  
things?

If my model is accurate, then I think commutation can stay the same,  
while the way this dependency is resolved changes.

When pulling "d/f" without "d" you get this dependency....

Normal
"d/f" --- pull in "adddir d" ---> "d"

Changed
"d/f" --- create local dir "d" (no pull) ---> "d"

Regards, Ben Coburn
msg7236 (view) Author: twb Date: 2009-01-28.01:10:30
On Wed, Jan 28, 2009 at 12:55:18AM -0000, Ben Coburn wrote:
>> Let me try again.  Is this modification you propose that:
>>
>>  adddir d and addfile d/f should commute
>>
>> I guess the result would be
>>
>>  addfile d/f and adddir d
>
> I do not think that I am proposing this, or that what I have
> described would require "adddir d" to commute with "addfile d/f".
>
> In my model of darcs I think of commutation and resolving
> dependencies as different things (that is sorting patches, and
> making sure the right patches exist in a repo). Is this the wrong
> way to conceptualize things?
>
> If my model is accurate, then I think commutation can stay the same,
> while the way this dependency is resolved changes.
>
> When pulling "d/f" without "d" you get this dependency....
>
> Normal
> "d/f" --- pull in "adddir d" ---> "d"
>
> Changed
> "d/f" --- create local dir "d" (no pull) ---> "d"

In other words, in Darcs 3.0 "adddir" becomes a deprecated noop patch
type, and therefore commutes with everything.  addfile and delfile
implicitly create/delete parent directories as needed.
msg7238 (view) Author: kowey Date: 2009-01-28.10:49:03
On Wed, Jan 28, 2009 at 00:55:18 -0000, Ben Coburn wrote:
> In my model of darcs I think of commutation and resolving dependencies  
> as different things (that is sorting patches, and making sure the  
> right patches exist in a repo). Is this the wrong way to conceptualize  
> things?

Unless somebody wiser in the way of patches shoots me down (and please
do): Given a sequence of patches p1 p2 (p2 applied after p1), we can say
that p2 depends on p1 if and only if the patches fail to commute.

It's part of the appeal of darcs (for me) and why we tend to think of
darcs as being actually rather simple.  It's all just commutation.
msg7276 (view) Author: btcoburn Date: 2009-02-05.06:33:53
See also issue1337.
msg8434 (view) Author: kowey Date: 2009-08-23.20:31:18
So we've ruled out changing commutation which we would need to do to change what
patches depend on what.  Marking this as wont-fix.  

I've split off Ganesh's suggestion in msg7230 in a new issue1559. 

More thoughts welcome, Ben :-)
History
Date User Action Args
2008-10-27 08:25:06btcoburncreate
2009-01-26 06:58:14twbsetstatus: unread -> unknown
nosy: + twb
messages: + msg7203
2009-01-26 07:22:50twbsettopic: + Confirmed
nosy: kowey, dagit, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
2009-01-27 00:01:37btcoburnsetnosy: kowey, dagit, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7216
2009-01-27 11:14:39koweysetnosy: kowey, dagit, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7218
2009-01-27 12:28:51twbsetnosy: kowey, dagit, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7222
2009-01-27 16:09:55btcoburnsetnosy: kowey, dagit, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7224
2009-01-27 16:42:21koweysetnosy: kowey, dagit, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7225
2009-01-27 23:26:41ganeshsetnosy: + ganesh
messages: + msg7227
2009-01-27 23:28:00ganeshsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7228
2009-01-28 00:06:21twbsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7229
2009-01-28 00:13:32ganeshsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7230
2009-01-28 00:15:29ganeshsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7231
2009-01-28 00:31:42twbsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7232
2009-01-28 00:34:04btcoburnsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7233
2009-01-28 00:48:00twbsetfiles: + accept-issue1177_-adddir-patches-result-in-silly-dependencies_.dpatch, unnamed
nosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7234
2009-01-28 00:55:18btcoburnsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7235
2009-01-28 01:10:32twbsetnosy: kowey, dagit, ganesh, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7236
2009-01-28 10:46:07koweysetnosy: + igloo
2009-01-28 10:49:06koweysetnosy: kowey, dagit, ganesh, igloo, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7238
2009-02-05 06:33:55btcoburnsetnosy: kowey, dagit, ganesh, igloo, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
messages: + msg7276
2009-08-10 23:49:35adminsetnosy: - dagit
2009-08-23 20:31:21koweysetstatus: unknown -> wont-fix
nosy: kowey, ganesh, igloo, simon, twb, thorkilnaur, btcoburn, dmitry.kurochkin
topic: - Confirmed
superseder: + forcible commutation maintaining patch identity
messages: + msg8434
2009-08-25 17:18:52adminsetnosy: + darcs-devel, - igloo
2009-08-25 17:32:45adminsetnosy: - simon
2009-08-27 14:31:18adminsetnosy: kowey, darcs-devel, ganesh, twb, thorkilnaur, btcoburn, dmitry.kurochkin