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

Release assets don't match repo structure #104

Closed
whilp opened this issue Oct 10, 2019 · 7 comments · Fixed by #443
Closed

Release assets don't match repo structure #104

whilp opened this issue Oct 10, 2019 · 7 comments · Fixed by #443
Assignees
Labels
p4 An idea that we are not considering working on at this time. wontfix

Comments

@whilp
Copy link

whilp commented Oct 10, 2019

The release assets helpfully produce tarballs that make recent releases look like old releases. This has avoided breaking users that update from release to release.

rules_pkg-0.2.4.tar.gz
$ curl -sLo -  https://github.com/bazelbuild/rules_pkg/releases/download/0.2.4/rules_pkg-0.2.4.tar.gz | tar tzf -
./
./BUILD
./README.md
./__init__.py
./archive.py
./build_tar.py
./build_zip.py
./deps.bzl
./helpers.py
./make_deb.py
./make_rpm.py
./path.bzl
./pkg.bzl
./rpm.bzl
./version.bzl
./releasing/
./releasing/BUILD
./releasing/__init__.py
./releasing/defs.bzl
./releasing/print_rel_notes.py
./releasing/release_tools.py

Unfortunately, that creates a divergence with the structure of the repo. Users that either follow commits on master or switch from releases to commits (to, say, get a particular fix before the next release) will observe strange-seeming issues like:

ERROR: error loading package '': Every .bzl file must have a corresponding package, but '@rules_pkg//:deps.bzl' does not have one. Please create a BUILD file in the same or any parent directory. Note that this BUILD file does not need to do anything except exist.
ERROR: error loading package '': Every .bzl file must have a corresponding package, but '@rules_pkg//:deps.bzl' does not have one. Please create a BUILD file in the same or any parent directory. Note that this BUILD file does not need to do anything except exist.

That's because :deps.bzl is actually pkg:deps.bzl in the repo.

This divergence seems unusual given my experience with other rules_* projects (go, nodejs, etc). With those projects, I am mostly able to flip between releases, commits, and my own forks. I can imagine that the history of rules_pkg makes that different or harder.

Is this expected? Do you intend to close the gap between release assets and the repo structure in the future?

Thanks!

@robbertvanginkel
Copy link

This also seems to have the effect that new rules don't get included in releases. For example,
https://github.com/bazelbuild/rules_pkg/releases/tag/0.2.5 mentions it adds a

Experimental rule set for better package content control (pkgfilegroup)

#128 added that rule to genpkg.bzl, with tests indicating it should be loaded like:

load("@rules_pkg//experimental:genpkg.bzl", "pkgfilegroup")

However, there is no experimental folder or genpkg.bzl in the release for which the release notes mention it was added:

$ curl -sLo -  https://github.com/bazelbuild/rules_pkg/releases/download/0.2.5/rules_pkg-0.2.5.tar.gz | tar tzf -
./
./BUILD
./README.md
./__init__.py
./archive.py
./build_tar.py
./build_zip.py
./deps.bzl
./helpers.py
./make_deb.py
./make_rpm.py
./path.bzl
./pkg.bzl
./rpm.bzl
./version.bzl
./releasing/
./releasing/BUILD
./releasing/__init__.py
./releasing/defs.bzl
./releasing/print_rel_notes.py
./releasing/release_tools.py
./releasing/release_tools_test.py
./releasing/update_changelog.py

cc @aiuto @nacl

@nacl nacl self-assigned this Mar 24, 2020
@nacl
Copy link
Collaborator

nacl commented Mar 24, 2020

Bah. Let me take a look.

nacl pushed a commit to nacl/rules_pkg that referenced this issue Mar 25, 2020
…/ tree

The release packages currently do not share the same file structure as the git
source tree, causing confusion when one switches from the git repository
structure to the archive structure.  Unfortunately, this doesn't really work
with rules_pkg right now due to bazelbuild/bazel#10062.

Further, the `experimental/` tree containing `pkgfilegroup` was missing from the
0.2.5 release archive.  It will be present in future releases (in
`pkg/experimental/`).

Additional changes in the future will involve changing the entire structure of
rules_pkg to match existing best practices (bazelbuild#111).  Given that this is somewhat
invasive change, this one is made for now to favor consistency with that
currently exists.

Testing was done by building the archive, extracting its contents, and
performing a diff (`//distro:rules_pkg-0.2.5`), further confirming that
`pkgfilegroup` was available with a custom project importing rules_pkg.

Fixes bazelbuild#104.
@aiuto
Copy link
Collaborator

aiuto commented Apr 14, 2020

The release does not match because we have two different projects inside this repo.

  • There are rules for consuming debian packages. AFAIK, no one is maintaining these. The
    docker rules may be the path forward there.
  • There rules under pkg for building packages of various types.

The later set is essentially a separate release set from the first (which actually has no releases).
I am explicitly packaging releases to be rules for consumption, so that they do not contain dependencies on things needed to develop the rules, such as stardoc. The releases will always be a subset of the repo source.

If we ever drop the debian package using rules, we could then lift the WORKSPACE from pkg up a level and make release and source identical. Getting the tar, deb, rpm, & zip rules to a better state is a far higher priority right now.

@aiuto aiuto added the wontfix label Apr 14, 2020
@blais
Copy link

blais commented May 31, 2020

This really needs to be documented on your front page. Ended up here after much confusion.
If you need to create two separate repos to normalize this, please do so. People's time is valuable.

@aiuto
Copy link
Collaborator

aiuto commented Jun 1, 2020

I want to understand user expectations and actual needs.
@whilp says:

This divergence seems unusual given my experience with > other rules_* projects (go, nodejs, etc). With those projects, I am mostly able to flip between releases, commits, and my own forks. I can imagine that the history of rules_pkg makes that different or harder.

I am used to the world of C software, where source trees exist, but people don't use the source in-situ. They build the source, install it somewhere, and use the result. Things get packaged. To me, having the source tree and the distribution package match structure is a strange constraint. In this case, I packaged the package rules so that the at user run time, the BUILD files should not depend on anything the user does not need. With explicit intent, we strip out the need to bring in stardoc or anything needed to build the distribution. In a more extreme case, imagine that we needed a code generator written in Kotlin. I can structure the distribution build so we bring in Kotlin only to generate something, then we use that in the final distro. It would be wrong to distribute files that could require a user to depend on kotlin, because they did something like bazel test //external/rules_pkg/....

Let's take the case of a casual user wanting to go right against git commits. I'm going to declare that is just unsupported. I want the right to keep head at an intermediate, unsupported state. The releases are what we publish.

The more interesting case is @whilp's of using your own fork. That is reasonable, because you take on the risk that you are using/making unsupported code, but the contract is explicit. For that, it seems like you could have the rules_pkg repo alongside your main workspace (or in the same SCM) with a local_repository rule that points into rules_pkg/pkg.

The question is what doc changes are needed.

  • Is this is just a matter of outlining a better cookbook of how to cope with the divergence for different use cases.
  • do I really need to say don't point directly to github commits? I may be showing my biases towards known code provenance by not even considering that - but if people really do that, I can add a warning against that.
  • does my example of the workflow for keeping your own dev branch work for you?

@statik
Copy link
Contributor

statik commented Jun 1, 2020

@aiuto the reason I needed to run a fork is because of bugfixes like #97 not being merged. My typical workflow for a fork is to fix an issue, get some real-world usage, and run on the fork while working with upstream to get the problem fixed.

Pointing to a github commit of my own fork containing bugfixes so that the artifacts produced are usable while waiting for upstream to respond is not abandoning code provenance.

Please do add the warning against pointing to git commits, it would have saved some headaches, as I would have realized that it was necessary to publish my own releases rather than pointing to a commit. I do not consider importing the rules_pkg repo into each consuming projects source tree and using local_repository a reasonable workaround.

@blais
Copy link

blais commented Jun 1, 2020

It's really simple: all the other projects work like that, and the way that this one doesn't manifests in a cryptic error message which, if you're not that familiar with Bazel, makes the user waste a lot of time in figuring out what's going on.

Two possible ways out of this:
a. Keep it as is - it's your right after all- but please write something on the front page telling users "If you get error message XX, it's because YY and here's how to use the project from head." (or maybe that you just cannot). Make it explicit. Because a lot of users will try to use it from head.
b. Give in and make it like all the other projects.

(a) will avoid people wasting their time (but still cannot use from HEAD).
(b) will make a lot more people use this seamlessly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4 An idea that we are not considering working on at this time. wontfix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants