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

How executable action can locate runfiles in both Windows and sandboxed execution #3726

Closed
alexeagle opened this issue Sep 12, 2017 · 16 comments

Comments

@alexeagle
Copy link
Contributor

Description of the problem / feature request / question:

Under windows, runfiles must be accessed via the runfiles_manifest since there are no symlinks.
However, this file is absent under the sandbox.

If possible, provide a minimal example to reproduce the problem:

https://gist.github.com/alexeagle/d7c57183f548c428469bec6ec4113419

$ bazel build :a
INFO: Found 1 target...
ERROR: /Users/alexeagle/Projects/repro_runfiles_manifest_missing/a/BUILD:9:1: SkylarkAction foo.txt failed (Exit 1).
hello world
writing to bazel-out/local-fastbuild/bin/foo.txt
/private/var/tmp/_bazel_alexeagle/4c2cb9588752d9f05411073c701a9ebc/bazel-sandbox/2423683567443745504/execroot/__main__/bazel-out/host/bin/sh.runfiles
/private/var/tmp/_bazel_alexeagle/4c2cb9588752d9f05411073c701a9ebc/bazel-sandbox/2423683567443745504/execroot/__main__/bazel-out/host/bin/sh.runfiles/__main__
/private/var/tmp/_bazel_alexeagle/4c2cb9588752d9f05411073c701a9ebc/bazel-sandbox/2423683567443745504/execroot/__main__/bazel-out/host/bin/sh.runfiles/__main__/repro.sh
/private/var/tmp/_bazel_alexeagle/4c2cb9588752d9f05411073c701a9ebc/bazel-sandbox/2423683567443745504/execroot/__main__/bazel-out/host/bin/sh.runfiles/__main__/sh
/private/var/tmp/_bazel_alexeagle/4c2cb9588752d9f05411073c701a9ebc/bazel-sandbox/2423683567443745504/execroot/__main__/bazel-out/host/bin/sh.runfiles/__main__/some_data
ls: /private/var/tmp/_bazel_alexeagle/4c2cb9588752d9f05411073c701a9ebc/bazel-sandbox/2423683567443745504/execroot/__main__/bazel-out/host/bin/sh.runfiles_manifest: No such file or directory
Target //:a failed to build

The find command in repro.sh lists all the files visible when the sandboxed action executes; the manifest file isn't present at all.

/cc @philwo

Environment info

  • Operating System:

  • Bazel version (output of bazel info release):
    $ bazel version
    Build label: 0.5.4-homebrew
    Build target: bazel-out/darwin_x86_64-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
    Build time: Fri Aug 25 16:55:29 2017 (1503680129)
    Build timestamp: 1503680129
    Build timestamp as int: 1503680129

  • If bazel info release returns "development version" or "(@non-git)", please tell us what source tree you compiled Bazel from; git commit hash is appreciated (git rev-parse HEAD):

Have you found anything relevant by searching the web?

(e.g. StackOverflow answers,
GitHub issues,
email threads on the bazel-discuss Google group)

Anything else, information or logs or outputs that would be helpful?

(If they are large, please upload as attachment or provide link).

@alexeagle
Copy link
Contributor Author

/cc @davidstanke this is a blocker for building typescript on windows

@philwo philwo self-assigned this Sep 13, 2017
@philwo philwo added category: sandboxing P1 I'll work on this now. (Assignee required) type: bug labels Sep 13, 2017
@philwo
Copy link
Member

philwo commented Sep 13, 2017

I will investigate and get back. Thank you very much for providing the repro case!

@philwo
Copy link
Member

philwo commented Sep 13, 2017

@ulfjack @dslomov It seems like the runfiles_manifest is not in the inputs of a Spawn, but is created (because when disabling sandboxing, the action in the repro example works).

Before I dive deeper into debugging this, do you have an idea out of your head why this is the case?

@dslomov
Copy link
Contributor

dslomov commented Sep 13, 2017 via email

@ulfjack
Copy link
Contributor

ulfjack commented Sep 14, 2017

The runfiles manifest is intentionally not in the inputs, like @dslomov says. This is WAI. We need to know what the actual problem is here.

@ulfjack ulfjack added more data needed and removed P1 I'll work on this now. (Assignee required) type: bug labels Sep 14, 2017
@alexeagle
Copy link
Contributor Author

The bigger problem is, how can an executable action locate its runtime dependencies? Symlinks are not present on Windows, and the manifest is not present in a sandbox. There seems to be no way remaining for the ctx.executable.compiler to find the some_data file in the repro above.

@ulfjack
Copy link
Contributor

ulfjack commented Sep 14, 2017

That run wasn't from a Windows run, and you didn't mention Windows above. We don't have sandboxing on Windows at this time, and we do have some tools to do runfiles lookups on Windows.

That said, I'm not sure we have adequately defined / designed a more permanent solution, but I'm sure it isn't making the current runfiles manifests (that contain absolute paths) available in the sandbox.

@alexeagle
Copy link
Contributor Author

I mentioned at the beginning of the description, "Under windows, runfiles must be accessed via the runfiles_manifest since there are no symlinks" but it could have been more clear:

I would like to write the compiler once, and run on all platforms. I think you are saying this is not possible, even for a compiler that isn't platform-dependent (running in NodeJS) it needs to have separate file resolution logic for Windows than on Mac/Linux. That's also true for wrapper scripts that bootstrap the compiler (eg. find its entry point).

What is the recommended pattern then, to write rules that work on all platforms? Look for a runfiles manifest file, and if not found, assume that the runfiles symlink tree is populated?

@alexeagle alexeagle reopened this Sep 14, 2017
@dslomov
Copy link
Contributor

dslomov commented Sep 14, 2017

What is the recommended pattern then, to write rules that work on all platforms? Look for a runfiles manifest file, and if not found, assume that the runfiles symlink tree is populated?

This will work for now (I appreciate it is not awesome)

That said, I'm not sure we have adequately defined / designed a more permanent solution, but I'm sure it isn't making the current runfiles manifests (that contain absolute paths) available in the sandbox.

We need to provide good guidance to people willing to have rules that work cross-platform. One possible solution is make sure people can always rely on the manifest being present. That would mean that inside the sandbox, the manifest will point to the files inside the sandbox (sandbox will generate that file).

@ulfjack
Copy link
Contributor

ulfjack commented Sep 14, 2017

I don't think we want to guarantee the existence of a runfiles manifest for every single action.

@ulfjack
Copy link
Contributor

ulfjack commented Sep 14, 2017

Well, ok, I'll upgrade that to a maybe. Although I'm still hopeful that Microsoft will make symlinks usable.

I don't think we'll be able to resolve it in this issue thread. If you need a short term solution, I'm afraid you'll have to go with separate logic on Windows vs. everything else.

@ulfjack
Copy link
Contributor

ulfjack commented Sep 14, 2017

I apologize for missing the Windows reference in the first sentence.

@alexeagle
Copy link
Contributor Author

alexeagle commented Sep 14, 2017 via email

@alexeagle alexeagle changed the title Runfiles manifest missing from sandboxed execution How executable action can locate runfiles in both Windows and sandboxed execution Sep 14, 2017
alexeagle added a commit to bazel-contrib/rules_nodejs that referenced this issue Sep 14, 2017
alexeagle added a commit to bazel-contrib/rules_nodejs that referenced this issue Sep 14, 2017
alexeagle added a commit to bazel-contrib/rules_nodejs that referenced this issue Sep 14, 2017
alexeagle added a commit to bazel-contrib/rules_nodejs that referenced this issue Sep 14, 2017
@eustas
Copy link

eustas commented Oct 10, 2017

I use $location substitution for (JVM) args for this. Works magically, until you try it on windows - bazel does not link data dependencies to .runfiles at all..

@alexeagle
Copy link
Contributor Author

alexeagle commented Oct 17, 2017 via email

@alexeagle
Copy link
Contributor Author

I think this could be closed in favor of #3889

devversion added a commit to devversion/angular that referenced this issue Jan 28, 2019
In order to support running "compiler-cli" tests that use the "test_support.ts"
utilities on Windows with Bazel, we need to imporve the logic that resolves NPM
packages and symlinks them into a temporary directory.

A more Bazel idiomatic and windows compatible way of resolving Bazel runfiles
is to use the "RUNFILES_MANIFEST" if present. This ensures that the NPM
packages can be also symlinked on Windows, and tests can execute properly
on Windows. Read more about why this is needed here:

* bazelbuild/bazel#3726 (comment)
matsko pushed a commit to angular/angular that referenced this issue Feb 5, 2019
In order to support running "compiler-cli" tests that use the "test_support.ts"
utilities on Windows with Bazel, we need to imporve the logic that resolves NPM
packages and symlinks them into a temporary directory.

A more Bazel idiomatic and windows compatible way of resolving Bazel runfiles
is to use the "RUNFILES_MANIFEST" if present. This ensures that the NPM
packages can be also symlinked on Windows, and tests can execute properly
on Windows. Read more about why this is needed here:

* bazelbuild/bazel#3726 (comment)

PR Close #28352
devversion added a commit to devversion/angular that referenced this issue Feb 5, 2019
In order to support running "compiler-cli" tests that use the "test_support.ts"
utilities on Windows with Bazel, we need to imporve the logic that resolves NPM
packages and symlinks them into a temporary directory.

A more Bazel idiomatic and windows compatible way of resolving Bazel runfiles
is to use the "RUNFILES_MANIFEST" if present. This ensures that the NPM
packages can be also symlinked on Windows, and tests can execute properly
on Windows. Read more about why this is needed here:

* bazelbuild/bazel#3726 (comment)
devversion added a commit to devversion/angular that referenced this issue Feb 5, 2019
In order to support running "compiler-cli" tests that use the "test_support.ts"
utilities on Windows with Bazel, we need to imporve the logic that resolves NPM
packages and symlinks them into a temporary directory.

A more Bazel idiomatic and windows compatible way of resolving Bazel runfiles
is to use the "RUNFILES_MANIFEST" if present. This ensures that the NPM
packages can be also symlinked on Windows, and tests can execute properly
on Windows. Read more about why this is needed here:

* bazelbuild/bazel#3726 (comment)
matsko pushed a commit to angular/angular that referenced this issue Feb 6, 2019
In order to support running "compiler-cli" tests that use the "test_support.ts"
utilities on Windows with Bazel, we need to imporve the logic that resolves NPM
packages and symlinks them into a temporary directory.

A more Bazel idiomatic and windows compatible way of resolving Bazel runfiles
is to use the "RUNFILES_MANIFEST" if present. This ensures that the NPM
packages can be also symlinked on Windows, and tests can execute properly
on Windows. Read more about why this is needed here:

* bazelbuild/bazel#3726 (comment)

PR Close #28550
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants