Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 6151 - Use %bcond macro for conditional builds in the spec file #6152

Merged
merged 1 commit into from
May 22, 2024

Conversation

vashirov
Copy link
Member

rpmbuild supports conditional package builds with the command line
switches --with and --without:
https://rpm-software-management.github.io/rpm/manual/conditionalbuilds.html

This is useful to rebuild an existing src.rpm file without editing the
spec file first. Or build in COPR with macros overrides for some
options. For example, automatic rebuilds from dist-git in COPR to
produced sanitized builds.

We use our custom global variables for different options such as
use_cockpit or use_asan. Instead we should switch to %bcond macro.

Fixes: #6151

@vashirov
Copy link
Member Author

There are some additional cleanups for the build system to make srpm generation easier, they are in separate commits to make the review easier.

@vashirov
Copy link
Member Author

Build failed in CI with:

python3 rpm/bundle-rust-npm.py src/ src/cockpit/389-console/ /__w/389-ds-base/389-ds-base/rpmbuild/SPECS/389-ds-base.spec -f
Traceback (most recent call last):
  File "/__w/389-ds-base/389-ds-base/rpm/bundle-rust-npm.py", line 23, in <module>
    from lib389.cli_base import setup_script_logger
ModuleNotFoundError: No module named 'lib389'
make: *** [rpm.mk:169: rpms] Error 1

rpm/bundle-rust-npm.py uses functions from lib389 module, which is not installed. I think our build system should not depend on the lib389 library. The code is trivial and I think we can just copy it to rpm/bundle-rust-npm.py. @droideck WDYT?

@vashirov vashirov changed the title Issue 6151 - Use %bcond macro for conditional builds in the spec file Issue 6151 - Use %bcond macro for conditional builds in the spec file Apr 19, 2024
Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides that, all looks good!

But there was a bit of something strange. I saw an issue with cargo-license error:
https://github.com/389ds/389-ds-base/actions/runs/8799564272/job/24148859806

python3 rpm/bundle-rust-npm.py src/ src/cockpit/389-console/ /__w/389-ds-base/389-ds-base/rpmbuild/SPECS/389-ds-base.spec -f
ERROR:bundle-rust-npm:cargo-license is not installed. Please install it with 'cargo install cargo-license' and try again.

But when I did a rerun, it disappeared (and the run after that). Can it be some weird race condition with containers?

print(f"Spec file {args.spec_file} is successfully modified! Please:")
print("1. Open the spec file with your editor of choice")
print("2. Make sure that 'Provides:' with bundled crates are correct")
print("3. Follow the instructions for 'License:' field and remove the helper comments")
Copy link
Member

@droideck droideck Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nitpick - we probably don't need this last line when we do a fix-it run.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. I have another series of changes coming up, one of them implements this :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@droideck, sorry for the delay, I pushed another series of commits, please take a look.

@vashirov
Copy link
Member Author

But when I did a rerun, it disappeared (and the run after that). Can it be some weird race condition with containers?

The container image didn't have cargo-license, I fixed it in 389ds/ci-images@c569be3. You probably had the old container image.

@vashirov vashirov requested a review from droideck May 16, 2024 10:50
Copy link
Member

@droideck droideck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Ack

`rpmbuild` supports conditional package builds with the command line
switches `--with` and `--without`:
https://rpm-software-management.github.io/rpm/manual/conditionalbuilds.html

This is useful to rebuild an existing src.rpm file without editing the
spec file first. Or build in COPR with macros overrides for some
options. For example, automatic rebuilds from dist-git in COPR to
produced sanitized builds.

We use our custom global variables for different options such as
`use_cockpit` or `use_asan`. Instead we should switch to `%bcond` macro.

Additional changes:
* `rpm/bundle-rust-npm.py`: add `-f` option to automatically do the changes
  to the `License:` field.
* Remove unneeded `389-ds-base-git.sh`
* Update `389-ds-base-devel.README`
* `rpm.mk`:
** exclude `vendor.tar.gz` from the resulting tarball
** add aliases for rpms and srpms targets
** use bundle-rust-npm.py with -f option for development releases
** add rpmspec target to generate a spec file under `rpm/` directory.
* `389-ds-base.spec.in`:
** remove unused/obsoleted lines
** remove `Provides:` for Rust crates, it will be populated by
   `bundle-rust-npm.py`
** add .asan to the NVR automatically
** use macro for nss Requires:
** move libdb globals behind if/endif

Fixes: 389ds#6151

Reviewed by: @droideck (Thanks!)
@vashirov vashirov merged commit 7df3957 into 389ds:main May 22, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use %bcond macro for conditional builds in the spec file
2 participants