docker / app

Make your Docker Compose applications reusable, and share them on Docker Hub
Apache License 2.0
1.57k stars 177 forks source link

Improve make -f docker.Makefile vendor #482

Closed simonferquel closed 5 years ago

simonferquel commented 5 years ago

- What I did

Initial work from @ijc brought a containerized vendoring which was great but had a few issues:

This PR fixes those issues.

- How I did it

Note that the dep-cache mount point and DEPCACHEDIR env var are //dep-cache and not /dep-cache. From a Unix FS point of view both paths are equivalent. But some windows shells (like the bash version shipped with Git on Windows) tries to do clever conversions to win32 paths when detecting unix-style paths in command line arguments, which makes docker trying to mount the volume in "c:\Program Files\Git\bin..." which makes no sense in a Linux container. Doubling the initial "/" disable this behavior.

- How to verify it

try make -f docker.Makefile on Windows

simonferquel commented 5 years ago

Yes, the exact reason why the target starts with an ugly docker rm -f docker-app-vendoring || echo "" I suppose I should also add a target for cleaning the vendoring cache volume (in case of a dep crash / an untimely docker rm -f). I'll add a commit with that.

codecov[bot] commented 5 years ago

Codecov Report

Merging #482 into master will decrease coverage by 0.12%. The diff coverage is 73.43%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #482      +/-   ##
=========================================
- Coverage   69.53%   69.4%   -0.13%     
=========================================
  Files          51      50       -1     
  Lines        2629    2553      -76     
=========================================
- Hits         1828    1772      -56     
+ Misses        569     545      -24     
- Partials      232     236       +4
Impacted Files Coverage Δ
internal/commands/inspect.go 78.37% <100%> (ø) :arrow_up:
internal/commands/uninstall.go 65.78% <50%> (-2.79%) :arrow_down:
internal/commands/status.go 71.42% <50%> (-3.58%) :arrow_down:
internal/commands/install.go 63.63% <50%> (-1.45%) :arrow_down:
internal/commands/upgrade.go 63.46% <70%> (-1.85%) :arrow_down:
internal/commands/cnab.go 68.99% <80.48%> (-10.21%) :arrow_down:
types/parameters/parameters.go 92.06% <0%> (-4.77%) :arrow_down:
internal/store/store.go 71.42% <0%> (-0.58%) :arrow_down:
internal/commands/dockerdesktop.go

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 622f5d9...2d70010. Read the comment docs.

ijc commented 5 years ago

Urk, I don't know wtf is going on with how those comments are presented (I did a per-commit review in different tags, but normally that DTRT, although I may have accidentally clicked "single comment" once or twice). Anyway, I think all of my comments have been captured in some form above.

simonferquel commented 5 years ago

@ijc @thaJeztah, just updated the PR. PTAL. I kept using the double "//" as the MSYS_* env var does not apply to my current mingw make environment... Imerged create/start into a single run, and I added the possibility to pass arguments (usefull for things like -update golang/x/sys etc.)