tilt-dev / tilt-extensions

Extensions for Tilt
https://tilt.dev/
Apache License 2.0
200 stars 156 forks source link

helm_remote enters infinite loop on Windows, with tilt_modules folder missing #572

Closed Venryx closed 3 months ago

Venryx commented 4 months ago

The logic for traversing up and finding the Tiltfile folder is hard-coded for Linux -- well, the part that ensures an infinite loop doesn't occur.

The function in question: (from: https://github.com/tilt-dev/tilt-extensions/blob/027c37bb3dc0ce11ee62eadac8c37d3742b53f6f/helm_remote/Tiltfile#L19C1-L28C41)

def _find_root_tiltfile_dir():
    # Find top-level Tilt path
    current = os.path.abspath('./')
    while current != '/':              # <-- this line
        if os.path.exists(os.path.join(current, 'tilt_modules')):
            return current

        current = os.path.dirname(current)

    fail('Could not find root Tiltfile')

Notice this line: while current != '/':

On Windows, this condition never evaluates to false, even after reaching the drive root, causing an infinite loop to occur if the "tilt_modules" folder doesn't exist (which is normal in my repo for a fresh clone; it is populated only after Tilt has had a chance to fully execute).

Note: Noticed this issue when a freelancer found tilt inexplicibly failing to run to the end of the script file; we identified this as the cause by using print-lines/bisection.

nicks commented 3 months ago

Hmmmm...i wasn't able to reproduce this problem.

Can you share what version of Tilt you're running?

Here's what I tried

1) Started a Windows box and opened a Cmd terminal 2) Opened the helm_remote tests 3) Updated the tiltfile to:

load('ext://helm_remote', 'helm_remote')

helm_remote('memcached', repo_url='https://charts.bitnami.com/bitnami')

docker_build('helm-remote-test-verify', '.')
k8s_yaml('job.yaml')
k8s_resource('helm-remote-test-verify', resource_deps=['memcached'])

And everything worked!

re: "the "tilt_modules" folder doesn't exist (which is normal in my repo for a fresh clone" -- that doesn't really make sense to me -- you can't even run this code unless you've downloaded it, and to downloading it puts it in a tilt_modules dir.

Venryx commented 3 months ago

re: "the "tilt_modules" folder doesn't exist (which is normal in my repo for a fresh clone" -- that doesn't really make sense to me -- you can't even run this code unless you've downloaded it, and to downloading it puts it in a tilt_modules dir.

Ah, that right there is the problem; I downloaded the helm_remote extension and added it to my project's repo under a different path, since I needed to make customizations to it: https://github.com/debate-map/app/tree/main/Tilt/%40Extensions

I still think the "don't keep looping forever" if statement should work for Windows as well, but if that check is considered redundant anyway (relying on the tilt_modules folder check instead), then I understand if this issue gets closed (I've already fixed my in-repo copy by adding another folder target / stop condition that is present in my repo even immediately after a fresh clone).

nicks commented 3 months ago

Fixed in https://github.com/tilt-dev/tilt-extensions/pull/573