Bugzilla (bugzilla.redhat.com) will be under maintenance for infrastructure upgrades and will not be available on July 31st between 12:30 AM - 05:30 AM UTC. We appreciate your understanding and patience. You can follow status.redhat.com for details.
Bug 1575257 - Review Request: llbuild - A low-level build system, used by Xcode 9 and the Swift Package Manager
Summary: Review Request: llbuild - A low-level build system, used by Xcode 9 and the S...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2018-05-05 13:07 UTC by Sascha Peilicke
Modified: 2020-07-21 09:54 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-07-21 09:54:33 UTC
Type: ---


Attachments (Terms of Use)

Description Sascha Peilicke 2018-05-05 13:07:58 UTC
Spec URL: https://copr-be.cloud.fedoraproject.org/results/saschpe/swift/fedora-28-x86_64/00750620-llbuild/llbuild.spec
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/saschpe/swift/fedora-28-x86_64/00750620-llbuild/llbuild-2018_04_25-1.fc28.src.rpm
Description:  llbuild is a set of libraries for building build systems. Unlike most build system projects which focus on the syntax for describing the build,  llbuild is designed around a reusable, flexible, and scalable general purpose build engine capable of solving many "build system"-like problems.  The project also includes additional libraries on top of that engine which provide support for constructing bespoke build systems (like swift build) or for building from Ninja manifests.

Fedora Account System Username: saschpe

https://koji.fedoraproject.org/koji/taskinfo?taskID=26789854
https://copr.fedorainfracloud.org/coprs/saschpe/swift/package/llbuild/

Comment 1 Robert-André Mauchin 🐧 2018-05-05 14:30:19 UTC
 - Not needed: Group: Development

 - Apache-2.0 is not a valid license shorthand, use "ASL 2.0". See full list: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#SoftwareLicenses

 - Use %global instead of %define:

%global rel swift-%{version}-RELEASE

   It should generally be located in the header for clarity

 - Also use a Release instead of a Development snapshots, latest is:

https://github.com/apple/swift-llbuild/releases/tag/swift-4.1.1-RELEASE

 - Split your BR one per line

BuildRequires: clang cmake ncurses-devel sqlite-devel

 - Spit your description to stay below 80 characters per line:

%description
llbuild is a set of libraries for building build systems. Unlike most build 
system projects which focus on the syntax for describing the build, llbuild is 
designed around a reusable, flexible, and scalable general purpose build engine 
capable of solving many "build system"-like problems. The project also includes 
additional libraries on top of that engine which provide support for
constructing bespoke build systems (like swift build) or for building from
Ninja manifests.

 - The licence file must be installed with %license not %doc:

%doc CONTRIBUTING.md README.md
%license LICENSE.txt

 - You're installing a library in %{_libdir}, you must run %ldconfig_scriptlets after %install

 - Add your own changelog entry:

%changelog
* Sat May 05 2018 Sascha Peilicke <sascha@peilicke.de> - 4.1.1-1
- Initial RPM release

 - pass build flags  to cmake, they need to be tweaked because of clang

%global fixed_set_build_flags echo '%set_build_flags' | sed 's/-mcet -fcf-protection//'
%fixed_set_build_flags

CC=$(which clang) CXX=$(which clang++) %__cmake .. \
            -DCMAKE_C_FLAGS_RELEASE:STRING="-DNDEBUG" \
            -DCMAKE_CXX_FLAGS_RELEASE:STRING="-DNDEBUG" \
            -DCMAKE_Fortran_FLAGS_RELEASE:STRING="-DNDEBUG" \
            -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON \
            -DCMAKE_INSTALL_PREFIX:PATH=%{_prefix} \
            -DINCLUDE_INSTALL_DIR:PATH=%{_includedir} \
            -DLIB_INSTALL_DIR:PATH=%{_libdir} \
            -DSYSCONF_INSTALL_DIR:PATH=%{_sysconfdir} \
            -DSHARE_INSTALL_PREFIX:PATH=%{_datadir} \
%if "%{?_lib}" == "lib64" 
            %{?_cmake_lib_suffix64} \
%endif
            -DBUILD_SHARED_LIBS:BOOL=ON

Comment 2 Neal Gompa 2018-05-05 16:34:40 UTC
Instead of reimplementing the %cmake macro just to add CC and CXX vars, you can just do:

export CC=clang
export CXX=clang++
%cmake ..

Comment 3 Robert-André Mauchin 🐧 2018-05-05 17:39:06 UTC
(In reply to Neal Gompa from comment #2)
> Instead of reimplementing the %cmake macro just to add CC and CXX vars, you
> can just do:
> 
> export CC=clang
> export CXX=clang++
> %cmake ..

The issue there is that the %cmake macro defines -mcet and -fcf-protection flags, whicha re incompatible with clang.

Comment 4 Robert-André Mauchin 🐧 2018-10-04 16:15:02 UTC
@Sascha Peilick
Latest clang may support  -mcet and -fcf-protection flags, so it may not be needed to redefine %cmake.

Are you still interested in packaging this?

Comment 5 Sascha Peilicke 2018-10-09 18:47:30 UTC
Maybe, but let's not bikeshed. I'd much prefer to get the git repository up and running. I can then try the macro again. I didn't expect contributing takes so much patience.

Comment 6 Zbigniew Jędrzejewski-Szmek 2019-04-21 16:22:54 UTC
Sascha, I see a few review requests from you, with unaddressed review comments (e.g. comment #1 above, https://bugzilla.redhat.com/show_bug.cgi?id=1575255#c5). Are you still planning to finish those?

Comment 7 Package Review 2020-07-10 00:56:36 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.


Note You need to log in before you can comment on or make changes to this bug.