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

Exclude files with spaces in bazel-managed node_modules #213

Closed
jwahlin opened this issue May 21, 2018 · 5 comments
Closed

Exclude files with spaces in bazel-managed node_modules #213

jwahlin opened this issue May 21, 2018 · 5 comments

Comments

@jwahlin
Copy link

jwahlin commented May 21, 2018

Several npm packages have spaces in file names. This breaks the bazel-managed node_modules. Adding a line like this to the exclude attribute should solve the problem:

"node_modules/**/* *.*",
@jwahlin
Copy link
Author

jwahlin commented May 29, 2018

I don't know how much flexibility you want to provide here, but if you want to expand the flexibility, you could add an exclude attribute to yarn_install and npm_install. Or, for even greater flexibility, you could add a build_file attribute.

@alexeagle
Copy link
Collaborator

Actually we want to move the other way and allow any files to be valid labels. I think this is already the case in the latest Bazel, what version are you on?

C:\Users\alexeagle\Projects\repro213
λ bazel query "deps(//:spaces)"
//:spaces
//:some file
//:WORKSPACE
//:BUILD

C:\Users\alexeagle\Projects\repro213
λ bazel info release
release 0.12.0

C:\Users\alexeagle\Projects\repro213
λ cat BUILD
filegroup(
    name = "spaces",
    srcs = glob(["*"]),
)

C:\Users\alexeagle\Projects\repro213
λ ls
 BUILD   WORKSPACE   bazel-bin@   bazel-genfiles@   bazel-out@   bazel-repro213@   bazel-testlogs@  'some file'

@jwahlin
Copy link
Author

jwahlin commented Jun 4, 2018

I'm on 0.13.0. I'll try the latest and see how it goes.

I've noticed you use self-managed node_modules in angular. Is that the approach you personally would recommend?

@alexeagle
Copy link
Collaborator

I think I am wrong, the repro above isn't sufficient to show that spaces aren't allowed in labels - @hansl had the same issue late last week.

Angular would like to switch to bazel-managed node_modules, I expect many projects would follow the same path of starting out more similar to web ecosystem, and as they adopt Bazel more, they'd be comfortable switching. The self-managed node_modules is really there to make the on-ramp simpler.

@gregmagolan
Copy link
Collaborator

Files with spaces are excluded in bazel-managed npm deps (those created with yarn_install or npm_install). The labels themselves are now legal in Bazel 0.17.1 but Bazel is not able to create a runfiles tree with these files as the MANIFEST file is space delimited (see bazelbuild/bazel#4327).

alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Oct 17, 2020
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this issue Oct 18, 2020
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

3 participants