Re: Conditional Patch line
On 06. 09. 22 19:06, Adam Williamson wrote: On Tue, 2022-09-06 at 09:04 +0100, Daniel P. Berrangé wrote: Unrelated to your question, but FWIW PatchNNN is not required, all patches can be merely "Patch: filename" and they'll get applied in the order they are listed in the spec. 勞勞勞 See https://lists.fedoraproject.org/archives/list/packag...@lists.fedoraproject.org/thread/BG5SVFAKGNVDJFHGZARABWGW2RRHN22G/ and https://youtu.be/0bfA0441dxc?t=666 and https://pagure.io/packaging-committee/pull-request/1157 -- Miro Hrončok -- Phone: +420777974800 IRC: mhroncok ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On Tue, 2022-09-06 at 09:04 +0100, Daniel P. Berrangé wrote: > > Unrelated to your question, but FWIW PatchNNN is not required, all > patches can be merely "Patch: filename" and they'll get applied > in the order they are listed in the spec. 勞勞勞 -- Adam Williamson Fedora QA IRC: adamw | Twitter: adamw_ha https://www.happyassassin.net ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On 05. 09. 22 21:58, Richard W.M. Jones wrote:
On Mon, Sep 05, 2022 at 09:56:58PM +0200, Dominik 'Rathann' Mierzejewski wrote:
On Monday, 05 September 2022 at 21:42, Richard W.M. Jones wrote:
I have a downstream patch[0] which -- I don't really understand why --
breaks riscv64 builds but is necessary for primary Fedora arches. Is
it correct to do:
%ifnarch riscv64
Patch123: downstream.patch
%endif
given that the package uses %autosetup and therefore doesn't have
explicit %patch lines.
I think this means that if I build the SRPM on riscv64 then the
downstream patch wouldn't be included, meaning that SRPM would then
fail to build on other arches.
This is correct. And forbidden by the packaging guidelines as others pointed
out.
As long as riscv64 lives in another Koji, this will not create real problems.
However once (if) riscv64 is added to the proper Fedora Koji, when the SRPM
would be built on riscv64, the build would fail on all other architectures due
to the missing patch file.
In this particular case that doesn't
matter, but it feels wrong. Is there a recommended way to do this
(apart from fixing the patch)?
Change %autosetup to plain %setup and apply the patch conditionally
instead of conditionally including it in the SRPM.
There are 26 patches so that's a bit of a PITA. Is there not an
easier way?
You can do this:
=
Patch1: ...
Patch2: ...
...
PatchX: ...
...
# Patches after 500 are applied conditionally
# Only applied on non-riscv64
Patch501: downstream.patch
...
%prep
%autosetup -N (-S git_am or whatever other options you like)
# apply all patches < 500
%autopatch -M 499
# apply conditional patches
%ifnarch riscv64
%autopatch 501
%endif
=
Alternatively, if you cannot (or don't want to) reorder the patches for some
reason, you could do:
=
# apply all patches <= 122
%autopatch -M 122
%ifnarch riscv64
%autopatch 123
%endif
# apply all patches >= 124
%autopatch -m 124
=
Which is a tad uglier.
Note that %autopatch accepts positional arguments with patch numbers only since
RPM 4.17. Before that you can either use %patch501 (which does not work with
%autosetup -S) or %apply_patch -q %{PATCH501} (which RPM considered internal
and removed from 4.17+).
For an uglier but portable solution that works on RPM <=4.16 and >=4.17, and
also respects %autosetup -S option, see
https://src.fedoraproject.org/rpms/python3.8/blob/f35/f/python3.8.spec#_736
--
Miro Hrončok
--
Phone: +420777974800
IRC: mhroncok
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
> On Mon, Sep 05, 2022 at 09:56:58PM +0200, Dominik 'Rathann' Mierzejewski > wrote: > > There are 26 patches so that's a bit of a PITA. Is there not an > easier way? > > Rich. Try using autopatch. # Apply patches up to #1000 from this spec. %autopatch -M1000 -p1 https://pkgs.rpmfusion.org/cgit/free/chromium-freeworld.git/tree/chromium-freeworld.spec#n222 ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On Mon, Sep 05, 2022 at 08:42:34PM +0100, Richard W.M. Jones wrote: > I have a downstream patch[0] which -- I don't really understand why -- > breaks riscv64 builds but is necessary for primary Fedora arches. Is > it correct to do: > > %ifnarch riscv64 > Patch123: downstream.patch > %endif > > given that the package uses %autosetup and therefore doesn't have > explicit %patch lines. Unrelated to your question, but FWIW PatchNNN is not required, all patches can be merely "Patch: filename" and they'll get applied in the order they are listed in the spec. > I think this means that if I build the SRPM on riscv64 then the > downstream patch wouldn't be included, meaning that SRPM would then > fail to build on other arches. In this particular case that doesn't > matter, but it feels wrong. Is there a recommended way to do this > (apart from fixing the patch)? Rather than fixing the root cause, if you want a relatively simple workaround still, you could merely move the conditional into the patch hack: if test $(uname -m) == "riscv64" then ...autoconf patch stuff... fi or something along those lines With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On Mon, 2022-09-05 at 15:00 -0500, Maxwell G via devel wrote:
> On Monday, September 5, 2022 Richard W.M. Jones wrote:
> > I have a downstream patch[0] which -- I don't really understand why
> > --
> > breaks riscv64 builds but is necessary for primary Fedora arches.
> > Is
> > it correct to do:
> >
> > %ifnarch riscv64
> > Patch123: downstream.patch
> > %endif
> >
> > given that the package uses %autosetup and therefore doesn't have
> > explicit %patch lines.
> >
> > I think this means that if I build the SRPM on riscv64 then the
> > downstream patch wouldn't be included, meaning that SRPM would then
> > fail to build on other arches. In this particular case that
> > doesn't
> > matter, but it feels wrong. Is there a recommended way to do this
> > (apart from fixing the patch)?
>
> Yes, conditionalizing Source or Patch lines is a bad idea. This exact
> case is
> explicitly forbidden by the guidelines[1]. There is also a guidelines
> PR to
> forbid any type of Source/Patch conditionaliztion[2].
because src.rpm will be different if build in one specific arch and
src.rpm should have the same results built in any arch
> [1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/
> #_no_arch_specific_sources_or_patches
>
> [2]: https://pagure.io/packaging-committee/pull-request/1163
>
> If you don't want to add %patchX for every Patch, you can use regular
> `%setup
> -q` together with %autopatch, which allows more granular control than
> `%autosetup`.
>
> From /usr/lib/rpm/macros:
>
> ```
> # Apply patches using %autosetup configured SCM.
> # Typically used with no arguments to apply all patches in the order
> # introduced in the spec, but alternatively can be used to apply
> indvidual
> # patches in arbitrary order by passing them as arguments.
> # -vVerbose
> # -p Prefix strip (ie patch -p argument)
> # -m Apply patches with number >= min only (if no
> arguments)
> # -M Apply patches with number <= max only (if no
> arguments)
> %autopatch(vp:m:M:) %{lua:
> ```
maybe this works:
%autosetup -M122 -m124
%ifnarch riscv64
%patch123 -p1
%endif
--
Sérgio M. B.
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On 5/9/22 16:42, Richard W.M. Jones wrote: I have a downstream patch[0] which -- I don't really understand why -- breaks riscv64 builds but is necessary for primary Fedora arches. Is it correct to do: %ifnarch riscv64 Patch123: downstream.patch %endif When I have to do things like that (often temporarily), I do something like this %ifnarch riscv64 Patch123: downstream.patch %else Source23: downstream.patch %endif given that the package uses %autosetup and therefore doesn't have explicit %patch lines. I think this means that if I build the SRPM on riscv64 then the downstream patch wouldn't be included, meaning that SRPM would then fail to build on other arches. In this particular case that doesn't matter, but it feels wrong. Is there a recommended way to do this (apart from fixing the patch)? Rich. [0] https://pagure.io/fedora-ocaml/c/41d5e2db7a4667560d6aedda11a3c6a80c8f1b83?branch=fedora-37-4.14.0 Pablo. ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On Monday, September 5, 2022 Richard W.M. Jones wrote:
> I have a downstream patch[0] which -- I don't really understand why --
> breaks riscv64 builds but is necessary for primary Fedora arches. Is
> it correct to do:
>
> %ifnarch riscv64
> Patch123: downstream.patch
> %endif
>
> given that the package uses %autosetup and therefore doesn't have
> explicit %patch lines.
>
> I think this means that if I build the SRPM on riscv64 then the
> downstream patch wouldn't be included, meaning that SRPM would then
> fail to build on other arches. In this particular case that doesn't
> matter, but it feels wrong. Is there a recommended way to do this
> (apart from fixing the patch)?
Yes, conditionalizing Source or Patch lines is a bad idea. This exact case is
explicitly forbidden by the guidelines[1]. There is also a guidelines PR to
forbid any type of Source/Patch conditionaliztion[2].
[1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/
#_no_arch_specific_sources_or_patches
[2]: https://pagure.io/packaging-committee/pull-request/1163
If you don't want to add %patchX for every Patch, you can use regular `%setup
-q` together with %autopatch, which allows more granular control than
`%autosetup`.
>From /usr/lib/rpm/macros:
```
# Apply patches using %autosetup configured SCM.
# Typically used with no arguments to apply all patches in the order
# introduced in the spec, but alternatively can be used to apply indvidual
# patches in arbitrary order by passing them as arguments.
# -vVerbose
# -p Prefix strip (ie patch -p argument)
# -m Apply patches with number >= min only (if no arguments)
# -M Apply patches with number <= max only (if no arguments)
%autopatch(vp:m:M:) %{lua:
```
--
Maxwell G (@gotmax23)
Pronouns: He/Him/His
signature.asc
Description: This is a digitally signed message part.
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
Do not reply to spam, report it:
https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On Mon, Sep 05, 2022 at 09:56:58PM +0200, Dominik 'Rathann' Mierzejewski wrote: > On Monday, 05 September 2022 at 21:42, Richard W.M. Jones wrote: > > I have a downstream patch[0] which -- I don't really understand why -- > > breaks riscv64 builds but is necessary for primary Fedora arches. Is > > it correct to do: > > > > %ifnarch riscv64 > > Patch123: downstream.patch > > %endif > > > > given that the package uses %autosetup and therefore doesn't have > > explicit %patch lines. > > > > I think this means that if I build the SRPM on riscv64 then the > > downstream patch wouldn't be included, meaning that SRPM would then > > fail to build on other arches. In this particular case that doesn't > > matter, but it feels wrong. Is there a recommended way to do this > > (apart from fixing the patch)? > > Change %autosetup to plain %setup and apply the patch conditionally > instead of conditionally including it in the SRPM. There are 26 patches so that's a bit of a PITA. Is there not an easier way? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
Re: Conditional Patch line
On Monday, 05 September 2022 at 21:42, Richard W.M. Jones wrote: > I have a downstream patch[0] which -- I don't really understand why -- > breaks riscv64 builds but is necessary for primary Fedora arches. Is > it correct to do: > > %ifnarch riscv64 > Patch123: downstream.patch > %endif > > given that the package uses %autosetup and therefore doesn't have > explicit %patch lines. > > I think this means that if I build the SRPM on riscv64 then the > downstream patch wouldn't be included, meaning that SRPM would then > fail to build on other arches. In this particular case that doesn't > matter, but it feels wrong. Is there a recommended way to do this > (apart from fixing the patch)? Change %autosetup to plain %setup and apply the patch conditionally instead of conditionally including it in the SRPM. Regards, Dominik -- Fedora https://getfedora.org | RPM Fusion http://rpmfusion.org There should be a science of discontent. People need hard times and oppression to develop psychic muscles. -- from "Collected Sayings of Muad'Dib" by the Princess Irulan ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
Conditional Patch line
I have a downstream patch[0] which -- I don't really understand why -- breaks riscv64 builds but is necessary for primary Fedora arches. Is it correct to do: %ifnarch riscv64 Patch123: downstream.patch %endif given that the package uses %autosetup and therefore doesn't have explicit %patch lines. I think this means that if I build the SRPM on riscv64 then the downstream patch wouldn't be included, meaning that SRPM would then fail to build on other arches. In this particular case that doesn't matter, but it feels wrong. Is there a recommended way to do this (apart from fixing the patch)? Rich. [0] https://pagure.io/fedora-ocaml/c/41d5e2db7a4667560d6aedda11a3c6a80c8f1b83?branch=fedora-37-4.14.0 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
