chore(concatjs_devserver): Updated io_bazel_rules_go dependency #2360
Conversation
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
| "strings" | ||
|
|
||
| "github.com/bazelbuild/rules_go/go/tools/bazel" | ||
| ) | ||
|
|
||
| // Runfile returns the base directory to the bazel runfiles | ||
| func Runfile(_base string, manifestPath string) (string, error) { |
There was a problem hiding this comment.
oh wow! I've been struggling to remove our rules_go patch on their runfiles library for a year. This is great!
There was a problem hiding this comment.
It might need a little more testing, but lets see if I get to test it over the holydays
KrauseStefan
force-pushed
the
concatjs_devserver--Updated-io_bazel_rules_go-dependency
branch
from
4ea4691 to
449f823
Compare
| // It seems like any npm dependency is referenced like 'cloud/../npm/node_modules/@angular/core/bundles/core.umd.js' and 'cloud/../npm/@bazel/typescript/third_party/npm/requirejs/require.js' | ||
| // This is true for both script dependencies and scriptsManifest | ||
| // If they are corrected to 'node_modules/@angular/core/bundles/core.umd.js' and '@bazel/typescript/third_party/npm/requirejs/require.js' the following if condition can be removed | ||
| if i := strings.Index(manifestPath, "/../npm"); i > 0 { |
There was a problem hiding this comment.
I don't think this is right in general, users can name this workspace whatever they like. "npm" is just the most common name
There was a problem hiding this comment.
Maybe you need to use ListRunfiles like Jay suggested back here: bazel-contrib/rules_go#2076 (comment)
There was a problem hiding this comment.
If using ListRunfiles I can also make it case insensitive. I don't believe web urls should not be case sensitive.
| patches = [ | ||
| # Patch out a breaking change to runfiles support library | ||
| # See discussion on https://github.com/bazelbuild/rules_go/pull/2076 | ||
| "@build_bazel_rules_typescript//:revert_rules_go_commit_4442d82a001f378d0605cbbca3fb529979a1c3a6.patch", |
There was a problem hiding this comment.
please delete the patch files as well since they should no longer be referenced
|
I think we need to update rules_go in order to fix the errors with bazel 4.0 upgrade in |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Using the old version
Issue Number: N/A
What is the new behavior?
Using the new version without the path
Does this PR introduce a breaking change?
Other information
White debugging some other issue I updated this dependency thinking that might solve the problem, turns out it did not but instead of throwing the code away I will would ask if you where interested in this update
References
bazelbuild/bazel#12744
bazel-contrib/rules_go#2076
CC @alexeagle