A lot of checks fail, but they don't seem to be related to my change.

It looks like we've got a real test failure:

    @Test
    public void shouldUseCopyOnlyWithReadOnlyClasspathResources() {
        String resource = "/test_copy_to_container.txt";
        GenericContainer<?> container = new GenericContainer<>()
            .withClasspathResourceMapping(resource, "/readOnly", BindMode.READ_ONLY)
            .withClasspathResourceMapping(resource, "/readOnlyNoSelinux", BindMode.READ_ONLY)

            .withClasspathResourceMapping(resource, "/readOnlyShared", BindMode.READ_ONLY, SelinuxContext.SHARED)
            .withClasspathResourceMapping(resource, "/readWrite", BindMode.READ_WRITE);

        Map<MountableFile, String> copyMap = container.getCopyToFileContainerPathMap();
FAILS-->assertTrue("uses copy for read-only", copyMap.containsValue("/readOnly"));
        assertTrue("uses copy for read-only and no Selinux", copyMap.containsValue("/readOnlyNoSelinux"));

        assertFalse("uses mount for read-only with Selinux", copyMap.containsValue("/readOnlyShared"));
        assertFalse("uses mount for read-write", copyMap.containsValue("/readWrite"));
    }

The copyToFileContainerPathMap is of type Map<MountableFile, String>. Now that it has equals/hashCode it behaves as a key should, and unfortunately copyToFileContainerPathMap ends up containing just one entry.

This doesn't seem right - it should be valid for one file to be copied to many places, so the original code seems to be wrong. It really feels like copyToFileContainerPathMap should be flipped around to be Map<String, MountableFile>. WDYT @bsideup ?

@rnorth: ... or it could be a Map<MountableFile, Set<String>>, which would be more clear about the intent.

Actually also the file mode should probably be part of the target, as the same file can end up at multiple locations with different modes, but I guess that would mean a breaking API change.

Perhaps we don’t actually need the container to be a map at all..? For our own usage we only care about iterating over the mount points, so this really just needs to be a list internally. De-duplication is not something we have to do; indeed currently we’ll just be letting Docker figure it out.

We don’t have a Triple class yet, but it would essentially be a List<Triple<MountableFile, String, BindMode>> (where the String is the destination path).

To avoid this being a breaking change we could keep the signature of getCopyToFileContainerPathMap as it is, but return an IdentityHashMap representation of the underlying List. I’m a bit sceptical that this method is useful for anything other than our unit tests, but we should avoid accidentally breaking it.

@t1 are you interested in continuing with this PR?

@rnorth : time is — as always — scarce... and this ticket has exploded. It's grown from implementing equals/hasCode to a redesign (even when it's just a small one). I don't think that I'll be able to squeeze out more time for this. My pain with the issue is just too small. It was more of a remove-some-cruft-issue to me.

Maybe it would be better to merge what we have than to wait for the full change?

@t1 sorry, but I think I'll close: as it is, there's a real test failure, so we can't merge it.

Similarly we're also very time constrained, so I too think that the effort of addressing the comments is not really worth it for a theoretical issue.

Thanks for your attempt to improve things, though.