Skip to content

loader: path resolution consistency#425

Merged
milas merged 2 commits intocompose-spec:masterfrom
milas:resolve-path-consistency
Jul 12, 2023
Merged

loader: path resolution consistency#425
milas merged 2 commits intocompose-spec:masterfrom
milas:resolve-path-consistency

Conversation

@milas
Copy link
Copy Markdown
Member

@milas milas commented Jun 28, 2023

Move the project working directory absolute path resolution to ResolveRelativePaths and remove redundant local volume (a rarely used way of creating bind mounts) absolute path resolution from the normalization method.

Move the project working directory absolute path resolution to
`ResolveRelativePaths` and remove redundant local volume (a
rarely used way of creating bind mounts) absolute path resolution
from the normalization method.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
@milas milas requested review from glours and ndeloof June 28, 2023 21:39
@milas milas self-assigned this Jun 28, 2023
Comment on lines -131 to -143
for name, config := range project.Volumes {
if config.Driver == "local" && config.DriverOpts["o"] == "bind" {
// This is actually a bind mount
config.DriverOpts["device"] = absPath(project.WorkingDir, config.DriverOpts["device"])
project.Volumes[name] = config
}
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic was duplicated in ResolveRelativePaths:

for name, config := range project.Volumes {
if config.Driver == "local" && config.DriverOpts["o"] == "bind" {
// This is actually a bind mount
config.DriverOpts["device"] = resolveMaybeUnixPath(project.WorkingDir, config.DriverOpts["device"])
project.Volumes[name] = config
}
}

N.B. It's actually SLIGHTLY different because the version in ResolveRelativePaths uses resolveMaybeUnixPath, which is arguably more correct.

@milas milas merged commit 2cef411 into compose-spec:master Jul 12, 2023
@milas milas deleted the resolve-path-consistency branch July 12, 2023 16:13
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

Successfully merging this pull request may close these issues.

3 participants