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

Nesting of pkg_filegroup is not possible #415

Closed
criemen opened this issue Aug 19, 2021 · 4 comments · Fixed by #420
Closed

Nesting of pkg_filegroup is not possible #415

criemen opened this issue Aug 19, 2021 · 4 comments · Fixed by #420
Labels
P3 An issue that we are not working on but will review quarterly

Comments

@criemen
Copy link

criemen commented Aug 19, 2021

I am using pkg_filegroup on sources that are a pkg_filegroup again.
This fails with

in srcs attribute of pkg_filegroup rule //:codeql-cpp: '//odasa-buildutils:dist-generic' does not have mandatory providers: 'PackageFilesInfo' or 'PackageDirsInfo' or 'PackageSymlinkInfo'.

Is there any specific reason that this is not allowed?
Using pkg_files everywhere (except at the top-level) is legal starlark, but needs very careful strip_prefix handling to not have too many/the wrong prefixes (or I don't understand strip_prefix well enough).

@nacl
Copy link
Collaborator

nacl commented Aug 19, 2021

Thanks for asking.

Is there any specific reason that this is not allowed?

I thought about doing this in the original pkg_filegroup design, but in discussions with @aiuto, we concluded that doing so would add too much complexity for a feature without immediate use. That aside, it should be doable.

Using pkg_files everywhere (except at the top-level) is legal starlark, but needs very careful strip_prefix handling to not have too many/the wrong prefixes (or I don't understand strip_prefix well enough).

Yeah, you do need to be careful with strip_prefix, and the behavior can be confusing or surprising at times. We're looking into making it more intuitive in #354.

I'm not opposed to implementing this in the long-term, and would be willing to look at patches to add such support.

@nacl nacl added the P3 An issue that we are not working on but will review quarterly label Aug 19, 2021
@aiuto
Copy link
Collaborator

aiuto commented Aug 19, 2021 via email

@criemen
Copy link
Author

criemen commented Aug 23, 2021

Okay, let me explain better what I want to achieve, with hopefully not too many internal terms.

I am building a prototype how to package what we call a CodeQL distribution (a zip file containing our software).
This is a zip archive that contains:

  • A base distribution (arch-specific, think compiled C binaries)
  • A base distribution (generic, think config files+java binaries)
  • 1..n language packs

Each language pack again can contain an arch-specific part and a generic part.

It makes sense for us to package up these language packs into zip files independent from the distribution, but also re-use the same pkg_filegroup to pack the distribution.
Thus, I want to map:

  1. Each part of the CodeQL distribution to a pkg_filegroup. These get joined together by a pkg_filegroup of filegroups into the main distribution (no prefix-stripping or anything going on, just taking the union of the filegroups).
  2. each languagepack can consist of a filegroup of arch-specifc and generic bits, as these usually need different prefix-munging applied to land in the correct paths

These filegroups can then serve as input to a bunch of pkg_zip targets, for different combinations.
Think: A full distribution/just a single language pack/a base distribution+a single language pack.

In the future, I'd also like to generate a pkg_tree instance for each of the zip targets, as we then can skip the zip/unzip part for the local development cycle.
For local development, we need the packaging step to move all the files to the correct paths, as our source paths and the paths in the released artifact are different, but we do not need to spend CPU cycles on actually packing/compressing anything.

Syntax-wise, I think you're there already, if I could use a pkg_filegroup label as srcs for another pkg_filegroup, and no extra path munging is applied. I.e. the top-level pkg_filegroup would just be a union over the pkg_filegroups specified as srcs.

@nacl
Copy link
Collaborator

nacl commented Sep 13, 2021

Thanks for the description. The idea makes sense, it's just a matter of implementing it.

We actually found a internal use case for this functionality, and a colleague of mine has an implementation available. A PR for it should be available soon.

@ghost ghost mentioned this issue Sep 14, 2021
aiuto pushed a commit that referenced this issue Sep 24, 2021
This commit enables users to feed a `pkg_filegroup` into another
`pkg_filegroup`.  Starlark tests included.

Resolves #415.

Testing Done:
- `bazelisk test tests/mappings/...`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 An issue that we are not working on but will review quarterly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants