Changes LayerEntry#extractionPath to use AbsoluteUnixPath. #1014
Conversation
…to i964-container.appRoot
|
Merge with master, and I should be able to look at this. |
| ImmutableList.builderWithExpectedSize(pathComponents.size() + relativePath.getNameCount()); | ||
| newPathComponents.addAll(pathComponents); | ||
| for (Path pathComponent : relativePath) { | ||
| if (pathComponent.toString().isEmpty()) { |
There was a problem hiding this comment.
You needed this when parsing String directly, but I don't think it's needed here. For example, System.out.println(Paths.get("///a///a///a//").getNameCount()) return 3.
There was a problem hiding this comment.
I'm using this mostly for Paths.get(""), which would return a nameCount of 1, but also as a just-in-case since it's not guaranteed that name elements are not empty for different implementations of Path.
There was a problem hiding this comment.
Huh, TIL about Paths.get("") having an empty filename.
There was a problem hiding this comment.
However, since resolving against a "" path doesn't add another name element, not having this in fromPath works.
There was a problem hiding this comment.
*having this in the call to fromPath in resolve(Path relativePath)
| } | ||
| newPathComponents.add(pathComponent.toString()); | ||
| } | ||
| return new AbsoluteUnixPath(newPathComponents.build()); |
There was a problem hiding this comment.
If going for simplicity, I guess AbsoluteUnixPath.fromPath(Paths.get(unixPath).resolve(relativePath)) will work? The code of fromPath() is quite similar. (We still need to check that relativePath is not absolute.)
| * @param relativePath the relative path to resolve against | ||
| * @return a new {@link AbsoluteUnixPath} representing the resolved path | ||
| */ | ||
| // TODO: Add test. |
There was a problem hiding this comment.
I think we should add tests in this PR, unless you plan to add these methods in another PR.
There was a problem hiding this comment.
Oops, I've already added tests and forgot to take this out.
jib-core/src/main/java/com/google/cloud/tools/jib/frontend/JavaDockerContextGenerator.java
Show resolved
Hide resolved
| @@ -177,7 +187,8 @@ public void testFilter_byNoEntries() throws CacheMetadataCorruptedException { | |||
|
|
|||
| LayerEntry fakeLayerEntry = | |||
| new LayerEntry( | |||
| Paths.get("some/source/file", "some/source/directory"), Paths.get("extractionPath")); | |||
| Paths.get("some/source/file", "some/source/directory"), | |||
There was a problem hiding this comment.
Unrelated, but this example seems weird, which just results in a path of /some/source/file/some/source/directory. Maybe the intention in the past was to pass two Paths as a list.
Fixes #1009