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.

Sign up for GitHub

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

Jump to bottom

Address --incompatible_restrict_string_escapes errors #1301

Closed
mmikitka opened this issue Oct 24, 2019 · 2 comments · Fixed by #2069
Closed

Address --incompatible_restrict_string_escapes errors #1301

mmikitka opened this issue Oct 24, 2019 · 2 comments · Fixed by #2069
Assignees
@mattem
Labels
cleanup Tech debt, resolving it improves our own velocity

Comments

@mmikitka
Copy link

mmikitka commented Oct 24, 2019 •
edited
Loading

Affected Rule

/build_bazel_rules_nodejs/internal/node/node.bzl

Is this a regression?

No. Compatibility with latest Bazel version.

Description

I'm seeing the following errors

external/build_bazel_rules_nodejs/internal/node/node.bzl:86:45: invalid escape sequence: \/. You can enable unknown escape sequences by passing the flag --incompatible_restrict_string_escapes=false

Minimal Reproduction

http_archive(
    name = "build_bazel_rules_nodejs",
    sha256 = "26c39450ce2d825abee5583a43733863098ed29d3cbaebf084ebaca59a21a1c8",
    urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/0.39.0/rules_nodejs-0.39.0.tar.gz"],
)

$ bazel version
Build label: 1.1.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Oct 21 08:44:00 2019 (1571647440)
Build timestamp: 1571647440
Build timestamp as int: 1571647440

Exception or Error

See above

Your Environment

See above
@alexeagle
Copy link
Collaborator

Hmm, it reads escaped = mn.replace("/", "\/").replace(".", "\.")
I guess this means the line is currently a no-op because the backslashes are treated as escape characters. So it should either be replace("/", "\\/") to restore the developers intent or just remove the line to preserve current semantics.

@alexeagle alexeagle added the cleanup Tech debt, resolving it improves our own velocity label Oct 24, 2019
@shahms shahms mentioned this issue Feb 19, 2020
Open
@mattem
Copy link
Collaborator

mattem commented Jul 23, 2020

This was addressed in #1747

@mattem mattem mentioned this issue Jul 23, 2020
Merged
@mattem mattem self-assigned this Jul 23, 2020
mattem added a commit that referenced this issue Jul 23, 2020
@mattem
build: set --incompatible_restrict_string_escapes and --incompatible_…
3af2449 
…no_implicit_file_export (#2069)

* build: changes to flip --incompatible_no_implicit_file_export #1508
* build: set --incompatible_restrict_string_escapes #1301
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattem mattem

Labels
cleanup Tech debt, resolving it improves our own velocity
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

build: set --incompatible_restrict_string_escapes mattem/rules_nodejs
3 participants
@alexeagle @mmikitka @mattem