Allow path options to use user specific paths #4852
Conversation
|
@laszlocsomor does this also work for Windows users? |
| @@ -103,7 +104,11 @@ public static String asFilteredShellEscapedString(OptionsProvider options) { | |||
|
|
|||
| @Override | |||
| public PathFragment convert(String input) { | |||
| return PathFragment.create(input); | |||
| String path = Preconditions.checkNotNull(input); | |||
| if (!path.isEmpty() && path.charAt(0) == '~') { | |||
There was a problem hiding this comment.
Please add an OS.getCurrent() check around this if, and only replace "~" on Linux/macOS.
There was a problem hiding this comment.
(OS in the same Java package)
There was a problem hiding this comment.
AFAIK, user.home should be resolved just fine on Windows and recent JDK versions: [1]. On Windows, on Java 8, we have:
public class Test {
public static void main(String[] args) {
String path = "~/foo/";
path = path.replace("~", System.getProperty("user.home"));
System.out.println(path);
}
}Running it produces:
C:\>java Test
C:\Users\davido/foo/FTR: I backported this solution from that Buck feature: Add support for user home in cache directory name that I added to Buck 4 years ago. And of course, Buck is platform independent.
[1] https://bugs.java.com/view_bug.do?bug_id=4787931
[2] https://stackoverflow.com/questions/2134338/java-user-home-is-being-set-to-userprofile-and-not-being-resolved
There was a problem hiding this comment.
Right, but ~abc is a valid directory name on Windows. So I'm reluctant to expand "~" to user.home on Windows too.
There was a problem hiding this comment.
We need a working and platform independent solution to configure local action path per default and commit it in (D)VCS. I am open to any suggestions, but we just can't solve the problem at hand for Linux/macOS, and ignore Windows users.
Should we compare for "~/" instead on all platforms, a lá:
if (path.startsWith("~/")) {
[...]?
There was a problem hiding this comment.
Yes please, that'd be better.
| @@ -123,6 +128,9 @@ public String getTypeDescription() { | |||
| List<PathFragment> list = new ArrayList<>(); | |||
| for (String piece : input.split(":")) { | |||
| if (!piece.isEmpty()) { | |||
| if (piece.charAt(0) == '~') { | |||
Fixes bazelbuild#2054. Allow users to be able to specify user specific paths. With this option we can now commit bazel configuration file and force local action cache activation per default: $ cat tools/bazel.rc build --experimental_local_disk_cache_path=~/.gerrit/bazel-cache/cas build --experimental_local_disk_cache build --experimental_strict_action_env Test Plan: $ bazel test //src/test/java/com/google/devtools/build/lib:util_test
davido
force-pushed
the
allow-path-options-to-use-home-directory-character
branch
from
cbe4ecb to
f05d535
Compare
|
@tomlu FYI |
During migration from Buck to Bazel we lost action caches activation per default. For one, local action cache wasn't implemented in Bazel, for another, there was no option to specify HOME directory. I fixed both problems and starting with Bazel 0.14.0 both features are included in released Bazel version: [1], [2]. There is still one not implemented option, limit the cache directory with max size: [3]. But for now the advantage to activate the caches per default far outweigh the disadvantage of unlimited growth of size of cache directory beyound imaginary max size of say 10 GB. In meantime we add a warning to watch the size of the directory cache and periodically clean cache directory: $ rm -rf ~/.gerritcodereview/bazel-cache/cas/* [1] https://bazel-review.googlesource.com/#/c/bazel/+/16810 [2] bazelbuild/bazel#4852 [3] bazelbuild/bazel#5139 Change-Id: I42e8f6fb9770a5976751ffef286c0fe80b75cf93
Fixes #2054.
Allow users to be able to specify user specific paths. With this option
we can now commit bazel configuration file and force local action cache
activation per default:
$ cat tools/bazel.rc
build --experimental_local_disk_cache_path=~/.gerrit/bazel-cache/cas
build --experimental_local_disk_cache
build --experimental_strict_action_env
Test Plan:
$ bazel test //src/test/java/com/google/devtools/build/lib:util_test