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

pkg_deb: allow passing deb and changes directly #263

Closed
wants to merge 1 commit into from

Conversation

amscanne
Copy link

(More detail in the commit.)

I've knocked this out quickly in anticipation of some feedback on a better way to solve this problem. Please let me know if I'm holding it wrong!

@amscanne amscanne requested a review from aiuto as a code owner December 10, 2020 05:21
@amscanne
Copy link
Author

This is actually broken in a non-trivial way, because the test depends on being able to refer to out_deb and out_changes explicitly in its runfiles. (Is this something that anyone does outside the test? I'm not sure.) I could change the test behavior, but I'd like to get guidance on this first.

Note that other people have hit this problem as well:
https://stackoverflow.com/questions/56190105/selecting-different-values-for-pkg-debs-package-and-architecture-attributes#new-answer

This allows for a proper select statement, since strict evaluation in
the macro makes it impossible.
@amscanne
Copy link
Author

Okay, I think I misunderstood the constraints. The latest patch just lets you pass a select for the architecture, so that you can at least work around the problem. The API for pkg_deb is otherwise unchanged.

@amscanne amscanne changed the title pkg_deb: construct out_deb within the rule pkg_deb: allow passing deb and changes directly Dec 10, 2020
copybara-service bot pushed a commit to google/gvisor that referenced this pull request Dec 30, 2020
This change works around an issue in rules_pkg, described here:
  bazelbuild/rules_pkg#263

PiperOrigin-RevId: 346720001
copybara-service bot pushed a commit to google/gvisor that referenced this pull request Jan 9, 2021
This change works around an issue in rules_pkg, described here:
  bazelbuild/rules_pkg#263

PiperOrigin-RevId: 350869030
@aiuto
Copy link
Collaborator

aiuto commented Jan 14, 2021

Can you start with an issue for this where we can come up with an overall design. The package_file_name attribute from pkg_tar will be available to pkg_dep too. That takes care of half this PR in a uniform way across rules.
Do we need that same capabilty for changes or is there a default format for changes which we should always use?

@matzipan
Copy link

AFAIU the problem is that you can't pass a select to architecture. And I think the proposed workaround was to pass debs and changes from the outside so this wouldn't be a blocker.

@aiuto
Copy link
Collaborator

aiuto commented Jan 28, 2021

I think #282 makes this obsolete. The net effect of that PR is that you can set changes explictly if you want. But if you don't, we use basename(package_file_name).changes. In turn, package_file_name might be built from the defaults or computed from things only known at build time (like cpu).

@aiuto
Copy link
Collaborator

aiuto commented Jan 29, 2021

Sorry, @matzipan, I didn't actually answer your question.

I don't understand why you are saying that architecture is not select()able. I just tried using a select for architecture, along with the defaulting for package_file_name and it just worked.

pkg_deb(
    name = "deb_select_arch",
    data = ":example1",
    description = "what it does",
    maintainer = "someone@somewhere.com",
    package = "foo_b",
    version = "1",
    architecture = select({
        ":special_build": "NewCPU",
        "//conditions:default": "x36",
    }),
)


$ bazel build :deb_select_arch
Target //:deb_select_arch up-to-date:
  bazel-bin/foo_b_1_x36.deb
$ bazel build :deb_select_arch --define=SPECIAL=1
Target //:deb_select_arch up-to-date:
  bazel-bin/foo_b_1_NewCPU.deb

Of course, looking at the stackoverflow question, I see that it might not have been selectable before #282, but all the output file computations are made more rationally now.

@matzipan
Copy link

Hi aiuto,

Thanks for the response. I was not aware of #282, which seems to address that. I think I've seen it in other Bazel extensions as well, so it's interesting that now I know how this can be fixed (I think it was rules_docker).

Andrei

@aiuto
Copy link
Collaborator

aiuto commented Feb 10, 2021

#282 is submitted now. This PR might be obsolete.

@amscanne
Copy link
Author

Based on your comment above, I’m happy and will close this issue. I’ll reopen if there’s something that isn’t working as expected.

copybara-service bot pushed a commit to google/gvisor that referenced this pull request Jan 28, 2022
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.

None yet

3 participants