tiltfile: fix ignores if context dir appears later

[liskin]

Consider the following (admittedly somewhat unusual) Tiltfile:

trigger_mode(TRIGGER_MODE_MANUAL)

clone_cmd = "test -e tilt-avatars || git clone https://github.com/tilt-dev/tilt-avatars"
custom_build(
    "x-clone-repo",
    deps = [],
    skips_local_docker = True,
    command = clone_cmd,
)
docker_build(
    "x-build-image",
    context = "tilt-avatars",
    dockerfile_contents = "\n".join([
        "FROM x-clone-repo",
        "FROM scratch",
        "COPY ./ /opt/app/",
    ]),
    ignore = [
        "**/.git",
    ],
)
k8s_custom_deploy(
    name = "x-dummy-resource",
    apply_cmd = "true",
    delete_cmd = "true",
    deps = [],
    image_deps = ["x-build-image"],
)

This implements on-demand git cloning — the clone happens during the build phase of the resource, and thus other builds can proceed concurrently and this may speed up overall tilt up (and it does in the quite complex tilt setup that we have).

In current Tilt, however, ignore and only are completely ignored if the context directory doesn't exist when the Tiltfile is loaded. This is quite a strange and seemingly pointless limitation, as everything else seems to work fine.

The fix is trivial — simply remove the IsDir check from dockerignoresFromPathsAndContextFilters. The code that follows the check handles the absence of the dockerignore files just fine, so it seems the IsDir check is simply forgotten there from previous iterations.

Note that a similar issue exists in the repoIgnoresForPaths function as well. The Tiltfile to reproduce that problem is the same, just without the ignore = ["**/.git"] bit. I'm not entirely sure if that isGitRepoBase check serves any real purpose — would there be any harm if ".git" was ignored unconditionally (well, we'd need to check for the empty path like dockerignoresFromPathsAndContextFilters does)?

Related: https://github.com/tilt-dev/tilt/issues/3048#issuecomment-836877339

[nicks]

hmmm...nice find!

i'm not sure this is the right fix. note that the integration tests are failing.

AFAIR the problem we ran into is that custom_build lets you depend on lots of individual files. and having lots of individual files in the ignore spec really blew up the matching engine and made everything slow.

what if we changed the logic to something like

generateIgnores := false
if isDir(path) {
  generateIgnores = true
} else if !exists(path) && isDockerBuild {
  generateIgnores = true
}

i think that would fix your use-case a bit more narrowly.

[nicks]

closing as stale