On Thu, Nov 28, 2024 at 12:34:50AM +0100, Kalev Lember wrote:
> On Thu, Nov 28, 2024 at 12:23 AM Kalev Lember <kalevlember@xxxxxxxxx> wrote:
>
> > On Wed, Nov 27, 2024 at 11:56 PM Fabio Valentini <decathorpe@xxxxxxxxx>
> > wrote:
> >
> >> On Wed, Nov 27, 2024 at 11:50 PM Zbigniew Jędrzejewski-Szmek
> >> <zbyszek@xxxxxxxxx> wrote:
> >> >
> >> > Hi Yaakov,
> >> >
> >> > I was looking to update rust-zram-generator and I noticed the following:
> >> >
> >> > commit 820c5ec20c000e2f0ef57d19970311901d598cf1
> >> > Author: Yaakov Selkowitz <yselkowi@xxxxxxxxxx>
> >> > Date: Mon May 15 20:13:52 2023 -0400
> >> >
> >> > Use vendored dependency in RHEL builds
> >> >
> >> > This introduces and rpm macro logic bug which causes the vendored
> >> dependencies
> >> > to be unpacked unconditionally. I guess that most likely the same
> >> pattern was
> >> > used in other packages. IIUC, the unpacked vendor/ dir is actually not
> >> used for
> >> > anything. But it seems wasteful and confusing to unpack the second
> >> sources.
> >> > It'd be nice to fix this everywhere the same pattern was used.
> >> >
> >> > The problem is this:
> >> >
> >> > %autosetup -n %{crate}-%{version_no_tilde} -p1
> >> %{?bundled_rust_deps:-a2}
> >> >
> >> > %{?bundled_rust_deps:…} effectively checks if %bundled_rust_deps is
> >> defined.
> >> > It always is, to either 0 or 1.
> >>
> >> I have noticed this too, for example here:
> >> https://src.fedoraproject.org/rpms/papers/blob/rawhide/f/papers.spec#_141
> >>
> >> The buggy %autosetup line seems to have been copied into spec files quite
> >> a lot.
> >>
> >> I fixed it (once) when I updated librsvg2 a few months ago, but forgot
> >> to check other places:
> >>
> >> https://src.fedoraproject.org/rpms/librsvg2/blob/rawhide/f/librsvg2.spec#_101-110
> >
> >
> > I actually noticed this when adding bundling to papers packaging and made
> > sure to leave bundled_rust_deps undefined for the non-bundled case, which
> > should fix this issue for papers. In my opinion, librsvg's alternative fix
> > is a little bit too verbose and repetitive - I quite like how Yaakov did it
> > originally (it's just that it was a tiny bit buggy).
> >
>
> I wonder if it would be better to use bconds there as that would avoid the
> pitfall of someone at a later time defining bundled_rust_deps to 0 and
> things not working right in that case? Something like,
>
> %bcond bundled_rust_deps %{defined rhel}
>
> %autosetup %{?with_bundled_rust_deps:-a1}
We'll have the vendoring patches applied in hundreds of packages,
since it's needed for RHEL. rust2rpm has native support for vendoring.
So I think the only maintainable option is to use that.
(I don't know how it works exactly… From a quick look, it does
the vendoring unconditionally when enabled. Maybe it needs to be
ehanced to make the vendoring optional à la the patches that Yaakov
was applying.)
Zbyszek
--
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
