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

Volume binding bug in windows #1338

Closed
giovannicuccu opened this issue Apr 23, 2020 · 1 comment · Fixed by #1347
Closed

Volume binding bug in windows #1338

giovannicuccu opened this issue Apr 23, 2020 · 1 comment · Fixed by #1347
Milestone

Comments

@giovannicuccu
Copy link

Description

I think volume binding in windows has a bug.
Consider this snippet

<volumes>
  <bind>
        <volume>${settings.localRepository}/../:/var/maven/.m2</volume>
   </bind>
</volumes>

on a windows pc it can resolve to C:\Users\giovanni.m2\repository/../
If you run the plugin with this volume configuration the container does not start and report File not found
If you use an explicit Windows path value like this one

<volumes>
  <bind>
        <volume>C:\Users\giovanni\.m2\:/var/maven/.m2</volume>
   </bind>
</volumes>

the container starts correctly
Looking at the code it seems that in class VolumeBindingUtil

public static String resolveRelativeVolumeBinding(File baseDir, String bindingString)

the code applies

resolvedFile.getCanonicalFile().getAbsolutePath()

only to relative paths

On windows the canonicalization operation it's needed even on absolute paths since it translates
C:\Users\giovanni.m2\repository/../ to C:\Users\giovanni.m2

Info

  • d-m-p version : all
  • Maven version (mvn -v) : 3.6.1

I don't think the problem is bound to a specific maven version

@rhuss
Copy link
Collaborator

rhuss commented Jun 4, 2020

Good find. Can you craft a PR to fix it ? That would be awesome !

@rohanKanojia rohanKanojia added this to the 0.35.0 milestone Mar 7, 2021
rohanKanojia pushed a commit to giovannicuccu/docker-maven-plugin that referenced this issue Apr 4, 2021
+ Fix VolumeBindingUtil to handle absolute windows paths as well

  Signed-off-by: Giovanni Cuccu <gcuccu@imolainformatica.it>
rohanKanojia pushed a commit that referenced this issue Apr 4, 2021
+ Fix VolumeBindingUtil to handle absolute windows paths as well

  Signed-off-by: Giovanni Cuccu <gcuccu@imolainformatica.it>
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 a pull request may close this issue.

3 participants