-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Use AbsoluteUnixPath for container.appRoot propagation #1012
Conversation
…to i964-container.appRoot
directoryInContext.resolve( | ||
Paths.get(copyDirective.extractionPath).relativize(layerEntry.getExtractionPath())); | ||
Path baseExtractionPath = Paths.get(copyDirective.extractionPath.toString()); | ||
Path relativeEntryPath = baseExtractionPath.relativize(layerEntry.getExtractionPath()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, maybe we can use copyDirective.extractionPath
directly and add a relativize
method to AbsoluteUnixPath
after we change LayerEntry#getExtractionPath
to return an AbsoluteUnixPath
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static final String DEFAULT_RELATIVE_CLASSES_PATH_ON_IMAGE = "classes/"; | ||
public static final String DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE = "libs/"; | ||
public static final RelativeUnixPath DEFAULT_RELATIVE_RESOURCES_PATH_ON_IMAGE = | ||
RelativeUnixPath.get("resources/"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove the trailing slash.
appRoot + DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE + "*"), | ||
appRoot.resolve(DEFAULT_RELATIVE_RESOURCES_PATH_ON_IMAGE).toString(), | ||
appRoot.resolve(DEFAULT_RELATIVE_CLASSES_PATH_ON_IMAGE).toString(), | ||
appRoot.resolve(DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE).toString() + "/*"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be appRoot.resolve(DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE).resolve(RelativeUnixPath.get("*")).toString()
to avoid the explicit string concatenation.
COPY resources /app/resources/ | ||
COPY classes /app/classes/ | ||
COPY exploded-war /exploded/war/ | ||
COPY libs /app/libs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we actually need a trailing slash.
From https://docs.docker.com/engine/reference/builder/#copy:
If <dest> does not end with a trailing slash, it will be considered a regular file and the contents of <src> will be written at <dest>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good catch.
Fixes #1011.