bugQuilt - Bugs: bug #46498, QUILT_PATCHES=.. doesn't work

 
 

bug #46498: QUILT_PATCHES=.. doesn't work

Submitter:  Jan Kratochvil <lace>
Submitted:  Sun 22 Nov 2015 09:39:03 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Invalid
Privacy:  Public Assigned to:  khali
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 30 Nov 2015 05:49:39 PM UTC, comment #10: 

As side notes, but this is getting off-topic:

  • Quilt is indeed not bound to a specific Linux distribution, even though it has been developed as SUSE for a long time and we use it heavily. It's not even Linux-specific, it runs on BSD, Solaris etc.
  • I do not have virtual machines with other distributions like you do. As an upstream maintainer, I expect bug reporters to do their home work and describe their problem in a way that I can understand and reproduce it on the systems I own or have access to.
  • I did not ask you for a reproducer that would be specific to my (SUSE) system. I asked for something generic, that would work on all systems, including mine. Your detailed explanations of what you are doing, were exactly what I needed, thank you.
Jean Delvare <khali>
Group administrator
Mon 30 Nov 2015 05:18:39 PM UTC, comment #9: 

Thanks for explaining your use case in details, that way I understand exactly what you are doing and why. That makes it easier for me to suggest a workaround.

First of all: this isn't a regression, in the sense that you are setting QUILT_PATCHES and QUILT_SERIES to unsupported values. Whether it worked before is irrelevant.

I understand the value of the trick of always applying patches and reverting them conditionally. We don't do that at SUSE though, and I wouldn't want to. The benefit doesn't seem worth the added complexity and performance cost. Ideally the source code should be the same for all distributions, and the source code, not the packaging system, should deal with the conditions.

Quilt indeed can't deal with such spec files, as it uses the patch name as a unique key to store original files before applying patches. It has always done this and this isn't going to change. Note that quilt does support %patch -R, just not on a patch which was already applied before. So again, that's not a bug in quilt.

I agree that it would be better to get "quilt setup" to generate valid series files, rather than editing them manually and tinkering with QUILT_PATCHES and QUILT_SERIES. If you want to be able to process such packages using "quilt setup", I would suggest the following approach:

For each patch which you need to revert conditionally, create a copy of the patch under an alternative name, and edit the header to say so. A different header is required so that the file has a different md5sum, as this is how "quilt setup" used to identify patches (more on this below.) Then in the spec file you would have:

Patch848: gdb-pahole-python2.patch
Patch849: gdb-pahole-python2-revert.patch

%patch848 -p1
%if 0%{!?el6:1}
%patch849 -p1 -R
%endif

From that spec file, "quilt setup" will generate a series file with:

gdb-pahole-python2.patch
gdb-pahole-python2-revert.patch -R

That would work just fine, without the need to set QUILT_PATCHES and QUILT_SERIES to strange values, nor to edit the series file manually.

Ideally you would be able to use a symbolic link for the revert patch, instead of a modified copy. However this requires:

  • A recent version of quilt, which supports "quilt setup --fast" and all its optimizations. Even v0.64 didn't have that so you would have to wait for v0.65 or use a git repository snapshot.
  • A version of rpm which passes uncompressed patch files to "patch" directly, instead of piping them. Old versions used to do that but this optimization was broken in rpm 4.6.0 and only fixed in 4.13.0, leaving a range of quilt-unfriendly versions.


So given these constraints I suppose that you are better with a modified copy for each revert patch as suggested earlier.

Jean Delvare <khali>
Group administrator
Thu 26 Nov 2015 05:01:14 PM UTC, comment #8: 

OK, you are right I have unusual quilt usage (at least since 2010 as I see).  This is because quilt does not support %patch -R:
  %patch848 -p1
  %if 0%{!?el6:1}
  %patch848 -p1 -R
  %endif
as for such case it generates in the series file:
  gdb-pahole-python2.patch
  gdb-pahole-python2.patch -R
which 'quilt push' cannot handle then.  This is why I manually edit the 'series' file after 'quilt setup' to remove the '.* -R' lines.  And I do not want to edit 'series' again and again.

Some patches need to be applied only on some distribution variants/versions.  And this apply+undo style is there so that all the patches are always at least once temporarily applied any any distro so that one discovers any patch getting out of sync (after a rebase for example) on any distro, not just on the distros where the patch is required.

So I see now that it would be better to fix the "-R" bug and use the series file put into the sourcetree by default.


"I am running openSUSE, not Fedora, so I can't test" - you said "I'm attaching a candidate fix, please give it a try." but that fix did not work. So I provided instructions for reproducibility of the problem. I have various VMs installed with OSes just for reproducing problems during upstream maintenance (GDB in my case). For upstream projects it is irrelevant what OS the maintainer is running himself/herself. At least Quilt seems to me to be a Linux project, not a SuSE project.

"please come up with something that is not distribution-specific." - OK, I could provide some snapshot of the rpm-built repository myself, that is up to further negotiation. I expected you have some Fedora VM ready like I have SuSE or Ubuntu VMs ready myself.

Jan Kratochvil <lace>
Wed 25 Nov 2015 05:46:42 PM UTC, comment #7: 

BTW, my proposed fix wasn't supposed to do what you asked for. I was indeed not clear about it, sorry for the confusion.

The purpose of my fix was that "quilt setup" keeps working even if the user's ~/.quiltrc defines QUILT_PATCHES and/or QUILT_SERIES. I found the bug while investigating the case you reported, but on second thought it is mostly independent from what you reported so I shouldn't have claimed it was a fix, and I shouldn't have attached it to this bug. I'll drop the reference from the patch description.

Jean Delvare <khali>
Group administrator
Wed 25 Nov 2015 05:45:34 PM UTC, comment #6: 

I am running openSUSE, not Fedora, so I can't test with fedpkg as you suggested. You reported a bug against quilt, not fedpkg, and I am maintaining quilt, not fedpkg. If you have a problem with fedpkg, report it to the fedpkg packager. If your problem is with quilt, and you want to provide reproduction steps (always a good idea, I agree), then please come up with something that is not distribution-specific.

When run on a spec file (and I suppose fedpkg is doing exactly that, right?), "quilt setup" leaves the patches outside of the source tree but creates the series file at the root of the source tree. This is on purpose: the series file is generated, like the source tree, so deleting it doesn't lose anything. The patches, OTOH must be preserved. This makes me wonder what exactly you are trying to achieve by setting QUILT_PATCHES and QUILT_SERIES the way you do. The patches are already outside the working tree without setting QUILT_PATCHES. And preserving the series file shouldn't be needed, as a future invocation of "quilt setup" will overwrite it anyway.

Maybe you can describe your work flow in more details, because at the moment I just do not understand why you would need to delete your source tree and extract it from the tarball manually again and again. When I adjust a patch set (and I do that on a regular basis), I do it with quilt, and it doesn't involve deleting the source tree.

Jean Delvare <khali>
Group administrator
Wed 25 Nov 2015 03:40:31 PM UTC, comment #5: 


> It does work as you can check with my patch.


No, it doesn't work. You carefully stripped the relevant part of my explanation in your quote so let me write it here again:

If you use option "-d workdir" for example, the "patches" symbolic link would point to "../.." instead. If the link is not created, and QUILT_PATCHES is set to "..", further quilt commands won't find the patches.

Demonstration (with your patch applied):

$ quilt setup -d workdir i2c-tools.spec
### md5sum: .b.....
### rpmbuild: tppppp
Unpacking archive i2c-tools-3.1.1.tar.bz2

$ cd workdir
$ ls
i2c-tools-3.1.1  series
$ cd i2c-tools-3.1.1
$ quilt push
Applying patch ../i2c-tools-r6250-ddr3-fix-ext-temp-refresh.diff
Patch ../i2c-tools-r6250-ddr3-fix-ext-temp-refresh.diff does not exist; applied empty patch

Now at patch ../i2c-tools-r6250-ddr3-fix-ext-temp-refresh.diff

So as you can see it does find the series file, but not the patches directory. Your "fix" caused the link not to be created, while it is definitely needed. When using option -d and/or option --sourcedir, the patches could be anywhere.

Jean Delvare <khali>
Group administrator
Mon 23 Nov 2015 07:48:37 PM UTC, comment #4: 

BTW it still works in CentOS-6 which uses:
  https://dl.fedoraproject.org/pub/epel/6/SRPMS/quilt-0.48-2.el6.src.rpm
There are no custom patches, just:
  quilt-0.48.tar.gz
(It had to work even with more recent quilts but that is not reproducible for me now, maybe I had also different ~/.quiltrc.)

Jan Kratochvil <lace>
Mon 23 Nov 2015 06:44:02 PM UTC, comment #3: 

I have tried file #35529 but I see no change (patched onlt quilt-0.64-2.fc23.noarch, not trunk).  I think you can also test that with Fedora 23: fedpkg clone -a gdb;cd gdb;fedpkg sources
I do not see also how it could provide the layout I use, that is to have both 'series' and all the patches out of the extracted tree.


> In the context of "quilt setup", setting QUILT_PATCHES to ".."
> can't work, because a symbolic link must be created to where
> the patches are, and the name of that link obviously can't be
> "..".


It does work as you can check with my patch.

> As far as I can see, the real bug here is that further quilt
> commands won't pick up the alternative patches and series
> names set by "quilt setup".


"quilt push", "quilt pop", "quilt add" and "quilt refresh" work, I do not remember other commands would not work.  Maybe they should work from arbitrary subdirectory, I am aware I need to run them from the top directory of the extracted sources tree.

Jan Kratochvil <lace>
Mon 23 Nov 2015 02:09:55 PM UTC, comment #2: 

I'm attaching a candidate fix, please give it a try.

(file #35529)

Jean Delvare <khali>
Group administrator
Mon 23 Nov 2015 12:51:43 PM UTC, comment #1: 

"quilt setup" has been checking that $QUILT_PATCHES doesn't exist since May 2004 and version 0.33. This is nothing new and Fedora 20 certainly had a more recent quilt package than this. I have no idea how QUILT_PATCHES=.. would work on Fedora 20, unless they had a custom patch to bypass the check.

The way you work makes some sense but it is uncommon and not guaranteed to work (read: not supported.) As documented in the manual page, QUILT_SERIES should be either a file name or an absolute file path. A relative file path isn't supported. The manual page isn't so clear about QUILT_PATCHES but the generally accepted idea is that, if you want your patches to live outside of the working tree, you should use a symbolic link pointing to them (as "quilt setup" does.)

In the context of "quilt setup", setting QUILT_PATCHES to ".." can't work, because a symbolic link must be created to where the patches are, and the name of that link obviously can't be "..". So at least for "quilt setup" QUILT_PATCHES is expected to be a directory name and not a path.

The workaround you propose works but only by accident. It happens that in the most common case, the "patches" symbolic link created by "quilt setup" will point to "..", so if the link isn't created but QUILT_PATCHES is set to "..", it all works fine. However if you use option "-d workdir" for example, the "patches" symbolic link would point to "../.." instead. If the link is not created, and QUILT_PATCHES is set to "..", further quilt commands won't find the patches.

As far as I can see, the real bug here is that further quilt commands won't pick up the alternative patches and series names set by "quilt setup". This is irrelevant of the values of the QUILT_PATCHES and QUILT_SERIES variables, what triggers the problem is that they are set in ~/.quiltrc at all (even if to the default values.)

I wrote a patch addressing this issue over 3 years ago but for some reason never sent it upstream. I'll do it now.

Jean Delvare <khali>
Group administrator
Sun 22 Nov 2015 09:39:03 PM UTC, original submission:  

I was always using:
$ cat ~/.quiltrc
QUILT_PATCHES=..
QUILT_SERIES=../series

BTW that seems logical to me, I want to keep the same series file across many
  rm -rf source;tar xzf source.tar.gz;cd source;quilt push ...
adjustments of the patchset.

It worked in some old Fedoras (20?) but at least since quilt-0.63-2.fc21 it regressed:
$ quilt setup ./gdb.spec
### md5sum: .[...].
### rpmbuild: tp[...]p
Unpacking archive gdb-7.6.1.tar.bz2
Directory gdb-7.6.1/.. exists
Trying alternative patches and series names...

I haven't investigated what commit regressed it but the fix seems to be simple to me.

Jan Kratochvil <lace>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #35529:  project-settings-have-priority.patch added by khali (2KiB - text/x-patch - Candidate fix)
file #35524:  quilt-setup.patch added by lace (432B - text/x-patch - The fix.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by khali (Posted a comment)
  • -email is unavailable- added by lace (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-11-30 khali SummaryQUILT_PATCHES=.. regression QUILT_PATCHES=.. doesn't work
    2015-11-30 khali StatusNeed Info Invalid
        Open/ClosedOpen Closed
    2015-11-23 khali Attached File- Added project-settings-have-priority.patch, #35529
        StatusConfirmed Need Info
    2015-11-23 khali StatusNone Confirmed
        Assigned toNone khali
    2015-11-22 lace Attached File- Added quilt-setup.patch, #35524

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code