Re: [Quilt-dev] [PATCH] quilt refresh: Add ability to not break symlinks

[Jason Wessel]

On 01/12/2015 06:37 AM, Jean Delvare wrote:
> Hi Jason,
> 
> On Wed, 7 Jan 2015 11:39:12 -0600, Jason Wessel wrote:
>> This is an 6 year old patch that is still actively used:
>>
>> http://lists.nongnu.org/archive/html/quilt-dev/2008-01/msg00004.html
>>
>> Revised again here:
>>
>> http://comments.gmane.org/gmane.comp.handhelds.openembedded/34224
>>
>> This patch was then updated to properly work with or without the
>> --backup option to the quilt refresh by using cat_to_new_file option
>> instead of mv for refresh. A reference to the QUILT_NO_RM_SYMLINKS was
>> also added to the usage information for quilt refresh.
> 
> Is there a legitimate use case for not setting QUILT_NO_RM_SYMLINKS?

Probably the most obvious is when what the link points to is not
writable.  For the Yocto Project or WR Linux where this patch to quilt
is employed that is certainly not relevant.

As you pointed out in your response, it is more about backward
compatibility.  I imagine some folks might actually be surprised if
their symlinks suddenly do not get replaced by a file because that is
how quilt has always worked.  Others would of course be happy.



> Quilt does not create symbolic links under patches, so if they are
> there, you must have created them on purpose and I can't think of any
> reason why you would want to break them. If that was the case, you
> would have created copies in the first place, or hard links.
> 
> I appreciate that you are preserving compatibility with the current
> behavior but I suspect that this behavior is accidental rather than
> desired. The "rm" in cat_to_new_file was originally added to solve the
> problem of hard-linked copies of patch files being modified by "quilt
> refresh". Calling "rm" breaks the hard link so other copies are left
> untouched.
> 
> I doubt that symbolic links were considered at that point (and this is
> why you have the problem you have.) So maybe it would be the right time
> to set a sane default behavior for them (and add option
> QUILT_RM_SYMLINKS later if anyone ever asks for it - but I suspect
> this won't happen.)
> 
> What do you think? Opinion anyone?



I am fine with changing the default, I have run with quilt set to
never remove my symlinks for many years at this point, and prior to
that had a script that restored the links which dates back to 2005.  I
have used quilt for many, many projects in conjunction with cvs, svn,
and git always using the symlinks.  I just thought it might be time to
actually try to get the logic integrated once and for all.  :-)


> 
>> (...)
>> @@ -324,7 +328,7 @@ if [ -e "$patch_file" ] && \
>>  then
>>      printf $"Patch %s is unchanged\n" "$(print_patch "$patch")"
>>  elif ( [ -z "$QUILT_BACKUP" -o ! -e "$patch_file" ] || \
>> -       mv "$patch_file" "$patch_file~" ) && \
>> +       cat_to_new_file "$patch_file~" < "$patch_file" ) && \
> 
> This comes with a small performance penalty in the general case. You
> probably won't notice it for a single patch, but people using the
> --refresh option of push or pop on large patch sets are likely to
> notice it. I wonder if this could be avoided, maybe by letting
> cat_to_new_file handle the backup by itself.


Or simply not use cat_to_new_file in the case where there is no link.
Initially, I opted for the most simple fix here, and I would be happy
to rework the patch to optimize the behavior. First we should decide
on an answer for the default behavior with symlinks.

Cheers,
Jason.