On Thu, Nov 28, 2024 at 12:23 AM Kalev Lember <kalevlem...@gmail.com> wrote:

> On Wed, Nov 27, 2024 at 11:56 PM Fabio Valentini <decatho...@gmail.com>
> wrote:
>
>> On Wed, Nov 27, 2024 at 11:50 PM Zbigniew Jędrzejewski-Szmek
>> <zbys...@in.waw.pl> wrote:
>> >
>> > Hi Yaakov,
>> >
>> > I was looking to update rust-zram-generator and I noticed the following:
>> >
>> >     commit 820c5ec20c000e2f0ef57d19970311901d598cf1
>> >     Author: Yaakov Selkowitz <yselk...@redhat.com>
>> >     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}

-- 
Kalev

>
-- 
_______________________________________________
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

Reply via email to