Change @rules_pkg// to @// to allow users to name the repo what they want.

[aiuto]

This allows users to include the rules and remap the repo to a new name
I used the @// to give a visual reminder that this is interpreted
w.r.t. to the repo, and not your workspace root.

This requires Bazel 2.x or above.

[nacl]

Out of curiosity, what is the change in 3.x that makes this possible? I can't seem to find it (glanced through the closed issue list and the git changelog).

[aiuto]

incompatible_remap_main_repo bazelbuild/bazel#7130 But that was a 2.x flip. I must have missed that. I'll mail the PR.

…

On Mon, Apr 27, 2020 at 2:56 PM Andrew Psaltis ***@***.***> wrote: Out of curiosity, what is the change in 3.x that makes this possible? I can't seem to find it (glanced through the closed issue list and the git changelog). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#154 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHFW43EM2OVF3OU3A6TROXIONANCNFSM4MI2LUUQ> .

[aiuto]

Since there is pressure for a new named release, I want to roll this into the code before doing that.

[nacl]

When I try to load this from a repo that very simply tests the experimental rpm packaging rules (via local_repository), I get errors like this:

ERROR: error loading package '': in /home/apsaltis/.cache/bazel/_bazel_apsaltis/604301d63bf9174c7d4397ed3b43f6f0/external/rules_pkg/experimental/rpm.bzl: Label '//experimental:pkg_filegroup.bzl' is invalid because 'experimental' is not a package; perhaps you meant to put the colon here: '//:experimental/pkg_filegroup.bzl'?

This is from within rules_pkg itself, so there may be something wrong with this change (or bazel, maybe). I'm not completely sure.

Reproduced using this example. WORKSPACE needs to be changed to point to your rules_pkg checkout (see README.md)

[nacl]

Copyrights should probably be added or updated everywhere. Maybe this should be done in another change?

[aiuto]

I think the best path is merge this with your goal of #142, and the source reorg, and make this version 0.3.0. I'll get back to it in a day or so.

[achew22]

@aiuto, forgive my naiveté, especially on a driveby, but isn't @// the path to the "main repo"? Wouldn't you want to change these to just // to use current-repo relative resolution?

Unfortunately I can't find better docs when grepping around about this than this

[aiuto]

Copyrights should probably be added or updated everywhere. Maybe this should be done in another change?

Copyrights do not have to be updated each year.

[aiuto]

@aiuto, forgive my naiveté, especially on a driveby, but isn't @// the path to the "main repo"? Wouldn't you want to change these to just // to use current-repo relative resolution?

Unfortunately I can't find better docs when grepping around about this than this

// and @// are the same. I was trying to signal something in path names but I'm less keen on that idea now. Reverted to '//'

[aiuto]

When I try to load this from a repo that very simply tests the experimental rpm packaging rules (via local_repository), I get errors like this:

That seems to happen if I run tests from experimental/tests/external_repo, but we shouldn't be doing that. We can't have multiple WORKSPACE files or things are too confusing. Integration tests that need a workspace will have to create their own as part of the test.

[aiuto]

When I try to load this from a repo that very simply tests the experimental rpm packaging rules (via local_repository), I get errors like this:

ERROR: error loading package '': in /home/apsaltis/.cache/bazel/_bazel_apsaltis/604301d63bf9174c7d4397ed3b43f6f0/external/rules_pkg/experimental/rpm.bzl: Label '//experimental:pkg_filegroup.bzl' is invalid because 'experimental' is not a package; perhaps you meant to put the colon here: '//:experimental/pkg_filegroup.bzl'?

This is from within rules_pkg itself, so there may be something wrong with this change (or bazel, maybe). I'm not completely sure.

Reproduced using this example. WORKSPACE needs to be changed to point to your rules_pkg checkout (see README.md)

bazel test ... works for me.
But your external_repo tests need to be changed. We can't have more than one WORKSPACE file in a tree or things become impossible to test.

[nacl]

Copyrights should probably be added or updated everywhere. Maybe this should be done in another change?

Copyrights do not have to be updated each year.

True. By "everywhere", I should have said "in the files that were changed for this PR."

[aiuto]

Copyrights should probably be added or updated everywhere. Maybe this should be done in another change?

Copyrights do not have to be updated each year.

True. By "everywhere", I should have said "in the files that were changed for this PR."

But they do not have to change in the files edited in this PR. A copy right notice need the year of first publication of the work. Changing copyright lines just because the file was touched creates needless churn.

[nacl]

But they do not have to change in the files edited in this PR. A copy right notice need the year of first publication of the work. Changing copyright lines just because the file was touched creates needless churn.

Fair enough. Thanks for the clarification.

[nacl]

When I try to load this from a repo that very simply tests the experimental rpm packaging rules (via local_repository), I get errors like this:

That seems to happen if I run tests from experimental/tests/external_repo, but we shouldn't be doing that. We can't have multiple WORKSPACE files or things are too confusing. Integration tests that need a workspace will have to create their own as part of the test.

The issue there is that this WORKSPACE assumes that there is a previously loaded workspace called "rules_pkg", and is simply there for demarcation purposes. It does not load rules_pkg on its own. My example actually tries to load rules_pkg as a local_repository

I'm happy to make changes to that experimental repository test regardless; it does need to be in some sort of external repository. What's your recommend way forward on this? Generating a test WORKSPACE like how pkg/releasing/release_tools.py does it?

[nacl]

When I try to load this from a repo that very simply tests the experimental rpm packaging rules (via local_repository), I get errors like this:

ERROR: error loading package '': in /home/apsaltis/.cache/bazel/_bazel_apsaltis/604301d63bf9174c7d4397ed3b43f6f0/external/rules_pkg/experimental/rpm.bzl: Label '//experimental:pkg_filegroup.bzl' is invalid because 'experimental' is not a package; perhaps you meant to put the colon here: '//:experimental/pkg_filegroup.bzl'?

This is from within rules_pkg itself, so there may be something wrong with this change (or bazel, maybe). I'm not completely sure.
Reproduced using this example. WORKSPACE needs to be changed to point to your rules_pkg checkout (see README.md)

bazel test ... works for me.

It does for me too. The issue was with external use of rules_pkg.

FWIW, the repro example works with HEAD on your topic branch. However, it didn't work the last time I tried it, or at a02e872 when I checked it out locally. It looks like the change from @// to // fixed the issue I was having.

I don't know if there's something wrong with my example, but it looks like @// is behaving differently than // here.