Re: small problem with vendored deps in rust packages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 28. 11. 24 0:34, Kalev Lember wrote:
On Thu, Nov 28, 2024 at 12:23 AM Kalev Lember <kalevlember@xxxxxxxxx <mailto:kalevlember@xxxxxxxxx>> wrote:

    On Wed, Nov 27, 2024 at 11:56 PM Fabio Valentini <decathorpe@xxxxxxxxx
    <mailto:decathorpe@xxxxxxxxx>> wrote:

        On Wed, Nov 27, 2024 at 11:50 PM Zbigniew Jędrzejewski-Szmek
        <zbyszek@xxxxxxxxx <mailto: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
        <mailto: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 <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 <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}

Yes please.

It's easier than %if 0%{?rhel} %global... 1 %else %global... 0 %endif

--
Miro Hrončok
--
Phone: +420777974800
Fedora Matrix: mhroncok

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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Fedora Announce]     [Fedora Users]     [Fedora Kernel]     [Fedora Testing]     [Fedora Formulas]     [Fedora PHP Devel]     [Kernel Development]     [Fedora Legacy]     [Fedora Maintainers]     [Fedora Desktop]     [PAM]     [Red Hat Development]     [Gimp]     [Yosemite News]

  Powered by Linux