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

Consider upstreaming to rules_go #3

Closed
fmeum opened this issue Dec 30, 2021 · 3 comments
Closed

Consider upstreaming to rules_go #3

fmeum opened this issue Dec 30, 2021 · 3 comments

Comments

@fmeum
Copy link
Contributor

fmeum commented Dec 30, 2021

I started a discussion in bazelbuild/rules_go#3036 to get this library upstreamed as @io_bazel_rules_go//go/runfiles. According to bazelbuild/rules_go#3036 (comment), this would require more definite upstream documentation on what a runfiles library should do. It probably makes sense to wait for bazelbuild/bazel#14336 and bazelbuild/bazel#14500 to be resolved first though.

@fmeum fmeum changed the title Upstream to rules_go Consider upstreaming to rules_go Dec 30, 2021
@phst
Copy link
Owner

phst commented Dec 30, 2021

Related: the discussion starting at bazelbuild/rules_go#2422 (comment)

more definite upstream documentation on what a runfiles library should do

I think that makes sense. Maybe file another bug against Bazel to track adding the documentation?

@fmeum
Copy link
Contributor Author

fmeum commented Dec 30, 2021

I can take care of that after the open issues have been resolved, which will make it easier to arrive at a definite version of that documentation.

(Not completely related, but do you perhaps happen to know someone I could @ to take a look at bazelbuild/bazel#14336?)

sluongng added a commit to sluongng/rules_go that referenced this issue Jun 17, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
sluongng added a commit to sluongng/rules_go that referenced this issue Jun 17, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
sluongng added a commit to sluongng/rules_go that referenced this issue Jun 17, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
sluongng added a commit to sluongng/rules_go that referenced this issue Jun 22, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
sluongng added a commit to sluongng/rules_go that referenced this issue Jun 23, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator
- Updated canonical path criterias
- Replaced usages of 'path' with 'path/filepath'

Note that some issues with windows still remain unresolved.

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
fmeum pushed a commit to fmeum/rules_go that referenced this issue Jul 27, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator
- Updated canonical path criterias
- Replaced usages of 'path' with 'path/filepath'

Note that some issues with windows still remain unresolved.

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
fmeum pushed a commit to fmeum/rules_go that referenced this issue Oct 13, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator
- Updated canonical path criterias
- Replaced usages of 'path' with 'path/filepath'

Note that some issues with windows still remain unresolved.

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
fmeum pushed a commit to sluongng/rules_go that referenced this issue Nov 1, 2022
Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator
- Updated canonical path criterias
- Replaced usages of 'path' with 'path/filepath'

Note that some issues with windows still remain unresolved.

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)
fmeum added a commit to bazelbuild/rules_go that referenced this issue Nov 6, 2022
* runfiles: port phst/runfiles to rules_go

Today rules_go provide //go/tools/bazel as the canonical runfiles
library for binaries to be executed with `bazel test` and `bazel run`.

However, the current implementation pre-date the recent changes in
Bazel's upstream.  Since then, all of the native runfiles library of
Bash, Java, CPP, Python have been refactored to follow a certain
convention in locating files. (1)

Although these are subjected to change with the incoming BzlMod feature,
it would be easier to maintain if we can keep rules_go's runfiles
library implementation aligned with native languages' implementation.

Today, it seems like https://github.com/phst/runfiles implemented
exactly that.  So with @fmeum suggestion and @phst permission (2), let's
port the newer, more accurate implementation to rules_go.

Future refactoring will mark the current exported APIs in
//go/tools/bazel as deprecated and/or swapping out the old implementation
underneath to use this newer package.

Changes in this PR included:
- Copy paste repository over
- Removal of .git and .gitignore and .githooks dir
- Removal of repository specific files: README, WORKSPACE
- Rename BUILD to BUILD.bazel
- Rename import path for both go and BUILD files
- Run gazelle over the packages
- Adjusted test cases to reflect new package paths
- Removed godoc related to installation instruction
- Fixed test to handle window path separator
- Updated canonical path criterias
- Replaced usages of 'path' with 'path/filepath'

Note that some issues with windows still remain unresolved.

(1): https://docs.google.com/document/d/e/2PACX-1vSDIrFnFvEYhKsCMdGdD40wZRBX3m3aZ5HhVj4CtHPmiXKDCxioTUbYsDydjKtFDAzER5eg7OjJWs3V/pub
(2): phst/runfiles#3 (comment)

* Consistently take in / and emit \ on Windows

* Fix runfiles tests on Windows

* Add deprecation note to old `Runfiles` method

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
@phst
Copy link
Owner

phst commented Nov 28, 2022

@phst phst closed this as completed Nov 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

No branches or pull requests

2 participants