fix link resolution for mounted volume on macos#1291
fix link resolution for mounted volume on macos#1291aholthagerty wants to merge 2 commits intocontainers:mainfrom
Conversation
updated call to os.path.realpath inside assert_volumes to be os.path.abspath Fixes containers#1290 Signed-off-by: Alan Holt <aholt@hagerty.com>
|
Looks good. Would it be possible to write an integration test for this? |
Signed-off-by: Alan Holt <aholt@hagerty.com>
aholthagerty
force-pushed
the
fix/issue_1290
branch
from
09db090 to
9ead6ec
Compare
Added! |
|
Anything that needs to be done to get this merged? I have other team mates that have been affected by this bug. |
|
I'm also affected, looking forward to this being merged. |
|
Pinging again. This bug still affects us, and this PR would fix it. |
|
Why is this not merged already? |
|
This has been open for almost 3 months now. is there anything needed on this? Or, what is the reason for the hold-up on merging? |
|
Going to just start commenting every week until I get some feedback or it gets merged. |
|
yep, I understand that it sucks being nagged. But we'd really appreciate that fix being applied ... |
|
Ping. |
|
Ping |
|
ping |
1 similar comment
|
ping |
|
@p12tic is there anything I can do to help move this along and get it merged? |
|
@mokibit maybe you'll know more about what is keeping this from being merged. I see you are a recent contributor. |
p12tic
left a comment
p12tic
left a comment
There was a problem hiding this comment.
Thanks for the pull request.
Sorry for the delay, it took so long because I did not have access to a mac. Now I do.
Best would be to setup macos unit tests, but this is separate.
Please add release note in newsfragments directory. Also I had several comments in the code.
| services: | ||
| web: | ||
| image: nopush/podman-compose-test | ||
| command: ["dumb-init", "/bin/busybox", "httpd", "-f", "-h", "/var/www/html", "-p", "8000"] |
There was a problem hiding this comment.
"echo", "hello" is enough. We don't actually end up testing httpd and it's fine if container shuts down after the test. Remove restart: always and other similar things that aren't really needed.
| @@ -0,0 +1,4 @@ | |||
| ``` | |||
There was a problem hiding this comment.
Remove - not needed. Integration tests are to be run through pytest/unittest.
|
|
||
| class TestPodmanCompose(unittest.TestCase, RunSubprocessMixin): | ||
| def test_with_docker_sock(self) -> None: | ||
| up_cmd = [ |
There was a problem hiding this comment.
inline into run_subprocess_assert_returncode command, same with down_cmd
| self.run_subprocess_assert_returncode(up_cmd) | ||
|
|
||
| finally: | ||
| out, _, return_code = self.run_subprocess(down_cmd) |
There was a problem hiding this comment.
run_subprocess_assert_returncode should be better in this case
5 participants
updated call to os.path.realpath inside assert_volumes to be os.path.abspath
Fixes #1290
Contributor Checklist:
If this PR adds a new feature that improves compatibility with docker-compose, please add a link
to the exact part of compose spec that the PR touches.
For any user-visible change please add a release note to newsfragments directory, e.g.
newsfragments/my_feature.feature. See newsfragments/README.md for more details.
All changes require additional unit tests.