Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow included/load'd files to use tilt extensions #3452

Closed
bobjackman opened this issue Jun 12, 2020 · 11 comments · Fixed by #3484
Closed

Allow included/load'd files to use tilt extensions #3452

bobjackman opened this issue Jun 12, 2020 · 11 comments · Fixed by #3484
Labels
enhancement New feature or request extensions https://docs.tilt.dev/extensions.html

Comments

@bobjackman
Copy link

bobjackman commented Jun 12, 2020

It's very helpful to be able to organize your tiltfile by separating sections out into smaller files that can be included into the main Tiltfile. However, those sub-files are unable to make use of tilt extensions. There are 2 ways we can fail at this:

Attempt, the first

Main Tiltfile

include('otherfile.tilt')  # this is just more tiltfile, NOT an extension, and NOT defining shared functions

Other Tiltfile

load('ext://someExtension', 'someFunction')
someFunction('doAThing')

This fails with: Error: cannot load ext://git_resource: extensions cannot be loaded from `load`ed Tiltfiles which is confusing, since this file was included, not loaded.

Ok, so let's load the extension first, and then include our sub-file...

Attempt, the second

Main Tiltfile

load('ext://someExtension', 'someFunction')  # load this first, so our included file can make use of it
include('otherfile.tilt')  # this is just more tiltfile, NOT an extension, and NOT defining shared functions

Other Tiltfile

someFunction('doAThing')

This one fails with: Error: /path/to/otherfile.tilt:1:1: undefined: someFunction which suggests the included file doesn't share global context with the main tiltfile.

*If included files aren't capable of using extensions, it creates a major limitation to organizing our scripts in this fashion.

@wu-victor wu-victor added the enhancement New feature or request label Jun 14, 2020
@bobjackman
Copy link
Author

@victorwuky is this relatively simple enough that it might land soon, or is it more likely a ways down the line?

@wu-victor wu-victor added the extensions https://docs.tilt.dev/extensions.html label Jun 15, 2020
@bobjackman
Copy link
Author

bobjackman commented Jun 15, 2020

@victorwuky for what it's worth, while it would definitely be nice for extensions to also load files, it seems to me like that's less important than the ability for non-extension, local includes to be able to make use of existing extensions.

@wu-victor
Copy link
Contributor

Thanks @kogi for the comment. Right now we don't have a clear timeline of when we'd pick up this feature in particular.

@maiamcc
Copy link
Contributor

maiamcc commented Jun 15, 2020

@kogi to give a little more context, it turns out this feature isn't actually that simple because (among other things) we'd need to do a lot of work around handling working directories and all the inevitable edge cases. So--we'll keep you posted!

@maiamcc maiamcc changed the title Allow included files to use tilt extensions Allow included/load'd files to use tilt extensions Jun 16, 2020
@nicks
Copy link
Member

nicks commented Jun 16, 2020

@maiamcc can you elaborate a bit more on why this is difficult to solve?

Ya, it seems like this should be much higher priority if it blocks the ability to use the restart_process extension

@nicks
Copy link
Member

nicks commented Jun 16, 2020

@maiamcc Your comment on tilt-dev/tilt-extensions#37 makes me think you might be confusing two different issues:

  1. load()-ing extensions from other extensions raises questions about how we version extensions, and handle diamond dependencies (extensions should be able to load other extensions #3472)
  2. load()-ing extensions from other sub-included files in the "normal" repo doesn't raise any versioning concerns, and is much easier to resolve

@bobjackman
Copy link
Author

bobjackman commented Jun 16, 2020

# 1 would be nice
# 2 is what this issue is asking for

@nicks
Copy link
Member

nicks commented Jun 19, 2020

@maiamcc how do you feel about removing the restart_process deprecation warning until this issue is resolved? seems like this issue is a blocker to adoption of restart_process.

@maiamcc
Copy link
Contributor

maiamcc commented Jun 19, 2020

Ugh yeah I was hoping it wouldn't be that hard to fix this issue, but since it's almost 3, I guess I'll revert the deprecation warning for today's release :-/

@maiamcc
Copy link
Contributor

maiamcc commented Jun 19, 2020

This should be fixed in the latest release -- let us know if you run into any trouble!

@maiamcc maiamcc closed this as completed Jun 19, 2020
@bobjackman
Copy link
Author

Woo! Works like a charm! Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request extensions https://docs.tilt.dev/extensions.html
Projects
None yet
4 participants