Skip to content

Changed the buildroot from BUILD to BUILDROOT - #108

Merged
aiuto merged 2 commits into
bazelbuild:masterfrom
mmikitka:change-buildroot-dir
Apr 17, 2020
Merged

aiuto merged 2 commits into
bazelbuild:masterfrom
mmikitka:change-buildroot-dir

Conversation

@mmikitka

Copy link
Copy Markdown
Contributor

PR for issue #107

@mmikitka
mmikitka requested a review from aiuto as a code owner October 23, 2019 23:50
@aiuto

aiuto commented Oct 28, 2019

Copy link
Copy Markdown
Collaborator

With this patched in I see errors like
Processing files: rules_pkg-1-0.x86_64
error: File not found: /tmp/tmpcpf0h45p/BUILDROOT/tests/testdata/a.ar
error: File not found: /tmp/tmpcpf0h45p/BUILDROOT/tests/testdata/a_ab.ar
error: File not found: /tmp/tmpcpf0h45p/BUILDROOT/tests/testdata/a_b.ar
error: File not found: /tmp/tmpcpf0h45p/BUILDROOT/tests/testdata/a_b_ab.ar
error: File not found: /tmp/tmpcpf0h45p/BUILDROOT/tests/testdata/ab.ar
error: File not found: /tmp/tmpcpf0h45p/BUILDROOT/tests/testdata/b.ar
error: File not found: /tmp/tmpcpf0h45p/BUILDROOT/tests/testdata/config

What version of rpmbuild are you using. I see
$ rpmbuild --version
RPM version 4.14.2.1

@mmikitka

mmikitka commented Nov 4, 2019

Copy link
Copy Markdown
Contributor Author

@aiuto I am unable to reproduce the test failures locally and I cannot find the link to the CI test results. Can you show me how to access the results?

Also, I'm running with rpmbuild version 4.14.1

@aiuto

aiuto commented Nov 8, 2019

Copy link
Copy Markdown
Collaborator

It was failing locally for me. There is no CI run yet.

@aiuto

aiuto commented Nov 8, 2019

Copy link
Copy Markdown
Collaborator

Maybe Python versions? What do you use? I would probably have tried with Python 3.7.5rc1.

FWIW, we'll be aggressively dropping Python2 support over the next 2 months. So, what I may do immediately is make the next rules_pkg release a breaking one where we require python2.

@mmikitka

Copy link
Copy Markdown
Contributor Author

@aiuto I fixed the build and test errors in my most recent commit

09:24:59 [mmikitka:~/src/mmikitka-rules_pkg/pkg] change-buildroot-dir(+0/-0) ± bazel build //...
INFO: Writing tracer profile to '/home/mmikitka/.cache/bazel/_bazel_mmikitka/d38ab51127fb322f13e31fff484bd624/command.profile.gz'
INFO: Analyzed 54 targets (0 packages loaded, 0 targets configured).
INFO: Found 54 targets...
INFO: Elapsed time: 0.057s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

09:25:03 [mmikitka:~/src/mmikitka-rules_pkg/pkg] change-buildroot-dir(+0/-0) ± bazel test //...
INFO: Writing tracer profile to '/home/mmikitka/.cache/bazel/_bazel_mmikitka/d38ab51127fb322f13e31fff484bd624/command.profile.gz'
INFO: Analyzed 54 targets (0 packages loaded, 0 targets configured).
INFO: Found 46 targets and 8 test targets...
INFO: Elapsed time: 0.080s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
//distro:packaging_test                                         (cached) PASSED in 5.1s
//releasing:release_tools_test                                  (cached) PASSED in 0.2s
//tests:archive_test                                            (cached) PASSED in 0.2s
//tests:build_test                                              (cached) PASSED in 0.5s
//tests:helpers_test                                            (cached) PASSED in 0.2s
//tests:make_rpm_test                                           (cached) PASSED in 0.1s
//tests:path_test                                               (cached) PASSED in 0.2s
//tests:zip_test                                                (cached) PASSED in 0.2s

Executed 0 out of 8 tests: 8 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see whINFO: Build completed successfully, 1 total action

Comment thread pkg/tests/test_rpm.spec
@mmikitka

mmikitka commented Jan 9, 2020

Copy link
Copy Markdown
Contributor Author

@aiuto Is there anything else that needs to be done to close this?

@MarkusTeufelberger

MarkusTeufelberger commented Feb 3, 2020 •

Copy link
Copy Markdown
Collaborator

The bug that's fixed in this PR broke our RPM builds too, would be great if this could be merged soon.

For comparison:
https://github.com/bazelbuild/bazel/blob/ed8a5ecffe83bf55da8ceacda47dba15dd02bd8a/tools/build_defs/pkg/make_rpm.py#L234-L247 uses BUILDROOT for building the package, but BUILD for copying the files.

In PR #75 this was changed to drop BUILDROOT and only use a single BUILD folder. Unfortunately rpmbuild wants a clean folder unless it is actually in the filesystem root folder - so it tends to delete + recreate its BUILDROOT folder right before the the %install section (https://unix.stackexchange.com/questions/10392/how-to-keep-the-content-of-the-buildroot).

Either now every spec file needs to contain this workaround, or the pkg_rpm call gets fixed.

@MarkusTeufelberger

Copy link
Copy Markdown
Collaborator

#129 by the way already contains this fix.

@aiuto aiuto left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK. I understand now. My bad on this. This history leading to the problem is

  • I spent most of my time with rpms at Google, where we don't write the %install section. We depend on rules which essentially do that, so all of our spec files look like
%install
%include %build_rpm_install

%files -f %build_rpm_files
  • Someone started with our internal RPM build to export to Bazel, ripping out the magic files.
  • In the tests the install locations are all aligned with the files section, so %install could be a no-op
  • When I started tweaking things, I missed the nuance.

So... this is all good.

@nacl is doing some work on the kind of features which would allow the rule to generate the %install and %files section for you. The importance of that is that the packages feeding RPM can also feed .deb or tar or zip, so that renaming, moving, and mode bits can be specified in one place, rather than for each rule.

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.

3 participants