Enable use of relative volume host-paths

[JonathonReinhart]

This enables the use of relative paths in a volume hostpath.

Previously, if one put this in .scuba.yml:

volumes:
    /foo: foo
    # or
    /foo:
        hostpath: foo

foo would be interpreted by Docker as a named volume, resulting in an empty directory at /foo, which is certainly not what the user intended.

Or, if one tried to work around that problem and do this:

volumes:
    /foo: ./foo

Then Docker would complain about the use of a relative path (a specifier containing a / but not starting with /), and suggest the use of an absolute path.

Now, when scuba encounters a relative path in a volume hostpath, it is interpreted as relative to the scuba root (the directory in which .scbua.yml is found). To avoid ambiguity with the old behavior, relative paths must start with ./ or ../.

I also considered interpreting relative paths as relative to the current working directory, but that seemed like a much less useful approach:

• The common scenario would be to mount a project folder somewhere else in the filesystem:

  volumes:
    /var/cualqomm/license:
        hostpath: ./cq_license

• I can't think of a scenario where I would want a bind mount (to a fixed container-path) to change depending on where my current directory is within my project tree.
  • If this was truly desired, one could use $PWD/foo.
• The scheme whereby relative paths in config files are relative to the directory containing the config file is intuitive and widely-used:
  • In Scuba
    • !from_yaml (ref)
  • In .gitconfig (ref)
  • In ADMan config (ref)
  • In Docker Compose:
    • env_file (ref)
    • extends.file (ref)
    • service volumes (ref)

Fixes #218

[github-actions]

Test Results

    5 files      5 suites   1m 52s ⏱️
169 tests 167 ✔️   2 💤 0 ❌
845 runs  835 ✔️ 10 💤 0 ❌

Results for commit d4a61bc.

♻️ This comment has been updated with latest results.

[JonathonReinhart]

I think I want to restrict the relative paths to starting with ./ (or ../), just like Docker Compose does.

While the long form is pretty unambiguous (given that it says hostpath):

volumes:
  /foo:
    hostpath: foo

The short form is currently (unintentionally, and undocumented) interpreted as a named volume.

volumes:
  /foo: foo  # Is this a relative path or a named volume?

Requiring it to start with a ./ (in both cases) makes it 100% clear it is a relative path.

I'd rather start with this strict approach and relax as necessary, rather than starting with it loosely defined and trying to tighten it up later.

[xanarin]

I think the behavioral choices here make a lot of sense. I agree that a named volume is not what I would expect from having a bare directory name, and I like that scuba does accept a bare directory name either and instead forces the user to use ./ or ../ to formally confirm their intention. I also agree that those relative paths should be based on the .scuba.yml directory. I think basing in on CWD would just be a footgun.

I think in the worst case, this feature will be made less frustrating to an unsuspecting user because of #195. If a non-existent relative host path is created, it will at least be created with permissions such that the user will easily be able to delete it without having to escalate to root. But I think that scenario is very unlikely since this is a newly-supported option.

[xanarin]

Overall, this looks great. A few small questions and documentation changes are all the useful feedback I can give. I also love the additional documentation on the exceptions that a function can raise! Between that and the new type annotations the codebase is definitely getting easier to spot-check and reason about.

[JonathonReinhart]

I also love the additional documentation on the exceptions that a function can raise!

You can thank the Google Python Style Guide for that 😃. (Not all of it I like though...)

Between that and the new type annotations the codebase is definitely getting easier to spot-check and reason about.

Yes! It's so much easier to comprehend, particularly in the case of complex dict or tuple types. Also, mypy is surprisingly fast (likely due to its cache), so especially in cases like this where there was lots of plubing / refatoring, much of the initial edit-save-check cycle was done with static analysis only. Then test_config.py which is pretty fast. And only then did I run test_main.py which is slow because it actually runs Docker.

With this, and some of the Python 2 tech debt paid off (#226), I'm pretty happy with the codebase these days.

[JonathonReinhart]

Thanks for the thorough review @xanarin!

[haboustak]

Unfortunately, this MR introduced a regression in some code I was recently reviewing. We had projects written in Go that were storing their GOMODCACHE between invocations of scuba into a go-mod-cache named volume.

For what it's worth, I'm not in support of this usage of scuba volumes, but there's at least one instance of existing code that made use of it., and if it was written once, it was pasted at least 4 more times.

As I understand the usage, the intent was to mount the named volume at /go/pkg/mod/cache inside the container.

ENV GOMODCACHE /go/pkg/mod/cache

# requires scuba 2.10.0
volumes:
  ${HOME}/.cache/go-build: ${HOME}/.cache/go-build
  /go/pkg/mod/cache: go-mod-cache

[JonathonReinhart]

which is certainly not what the user intended.

Well that will teach me not to assume I understand my users.

To avoid ambiguity with the old behavior, relative paths must start with ./ or ../.

Whew, looks like this was a good idea.

Thanks for pointing this out. I'll open a bug to track the fix.

[JonathonReinhart]

Opened #248.