Add a Debug Terminal launcher which allows to launch a terminal into environment of an application

BigRoy commented 3 months ago

Changelog Description

Implements a Terminal (aka command line or "shell") launcher action:

When clicked allows you to pick the shell context to enter (For which application):

From that shell (when launched into Maya app for example) I can directly type mayapy to launch Maya within the environment of the Maya application:

Additional info

Note: currently always runs cmd (Windows command prompt) which for now makes this Windows only when testing. We'll need to implement the following TODO to solve that:

[x] Expose 'defaults' for the terminal to use in studio settings
- This could be a Terminal application. Or are there reasons not to do that @iLLiCiTiT?
- Other approaches to figuring out the best 'shell' to launch for a user is more than welcome. @dee-ynput :)
[x] Allow user-specific overrides for the terminal to use?
- What type of settings would that need?
[x] Use the first matching executable for current platform as defined by settings

Testing notes:

Launch tray
Use the launcher action

AY-1261

iLLiCiTiT commented 3 months ago

I can do code review, but don't understand the use-case so I can't review that part.

dee-ynput commented 3 months ago

Expose 'defaults' for the terminal to use in studio settings

This could be a Terminal application. Or are there reasons not to do that @iLLiCiTiT?

This sounds appropriate. But wouldn't that create a loop? Like opening a shell in the environment of a shell 😃

- Other approaches to figuring out the best 'shell' to launch for a user is more than welcome. @dee-ynput :)
If we manage to have it defined as an identified variant of a "Terminal" app I think we cover all our needs (platform executable, extra env, and cli args).

BigRoy commented 3 months ago

This sounds appropriate. But wouldn't that create a loop? Like opening a shell in the environment of a shell 😃

I'd say it'd then open a shell like:

Get terminal env
Overlay application env
Launch the terminal with that env

If we manage to have it defined as an identified variant of a "Terminal" app I think we cover all our needs (platform executable, extra env, and cli args).

Does this also automatically expose this as user-specific overrides somewhere? I can't recall seeing that. Say one particular dev wanted to use a different terminal app.

iLLiCiTiT commented 3 months ago

This could be a Terminal application. Or are there reasons not to do that @iLLiCiTiT?

Not sure what you mean by Terminal application, but each OS have way how to get default terminal. On windows can be used wt.exe (and if is not found then use cmd.exe). On macOs and linux it should be defined under env variable SHELL (this needs validation).

NOTE: macOs requires ["open", "-na", os.getenv("SHELL"), subprocess.list2cmdline(args)].

BigRoy commented 3 months ago

[x] We may need a Mac and Linux test here as well. If that works out of the box, then making the 'terminal' itself configurable for a specific user can be a follow up PR. @Lypsolon Linux? :) And who can test on Mac?
- [x] Windows
- [x] Linux
- [ ] Mac
[ ] Allow to enable it only for admin or only in dev mode via settings (however, LauncherActions currently don't receive any project settings so querying that each time may be a bit slow?) We may also want to make that a separate "feature" and hence separate PR. @iLLiCiTiT thoughts? I'd also argue that it'll always appear near the end of the list due to the order 10 and the client who requested a feature like this may be a TD who's not necessarily "admin" in the project either?

m-u-r-p-h-y commented 3 months ago

Or just an idea to treat it as a normal App, which can be enabled, disabled, configured, having variants for different shells (cmd, powershell, bash, tcsh etc.)

BigRoy commented 3 months ago

Or just an idea to treat it as a normal App, which can be enabled, disabled, configured, having variants for different shells (cmd, powershell, bash, tcsh etc.)

Yes - that's what @dee-ynput proposed, but that wouldn't technically allow it to be overridden per user, right? Say, one dev prefers a different terminal.

I would prefer it to be:

Studio provides defaults
User allows to override to pick another shell if they really want.

MustafaJafar commented 3 months ago

If we manage to have it defined as an identified variant of a "Terminal" app I think we cover all our needs (platform executable, extra env, and cli args).

Should that setting exists in core settings beneath Tools/Launcher Actions/Debug Shell

in studio settings with project overrides ? (won't be per user)
OR, in core site settings ? (per site id(machine), but no project overrides).

OR

Should it fetch the executable from a terminal app ? (which adds one more app in the launcher UI ,and not per user)

MustafaJafar commented 3 months ago

I would prefer it to be:

* Studio provides defaults

* User allows to override to pick another shell if they really want.

I like this idea but, it will make the debug action works in two step:

select your favorite shell/terminal
select your favorite app to debug

BigRoy commented 3 months ago

2. select your favorite app to debug

Not if that would be a user specific setting in the frontend, like a site-override or whatever. Didn't we have user settings? (I guess we don't since we never query settings with user as an argument)

MustafaJafar commented 3 months ago

Not if that would be a user specific setting in the frontend, like a site-override or whatever. Didn't we have user settings?

let me quote: we only have 4 scopes of settings:

studio settings: achieved by specifying studio in scope list.
project settings: achieved by specifying project in scope list.
project site settings: achieved by specifying site in scope list.
studio site settings: Currently achieved by adding settings to site_settings_model attribute in the addon class

Notes about Site Settings

In general, they are local settings for each site id. i.e. for every connected machine as every machine gets a unique site id.
They don't inherit (nor override) any other settings. i.e. project site settings are from studio site settings

BigRoy commented 3 months ago

I've now added dedicated "shell" application settings and use that to launch the terminal/console/shell with the application's environment. (It should actually, I believe - also apply the application env as defined in applications/shell)

Note that it's hardcoded for now to use the shell/main entry as I wasn't sure how to best allow an artist to pick the particular version they want to launch of the shell terminal. Maybe what @MustafaJafar explained would be best - that if there are multiple variants configured that it'd just list them all and allow you to choose - otherwise if there's just one, use that. If there's none... then it'd error?

@dee-ynput @iLLiCiTiT thoughts?

m-u-r-p-h-y commented 3 months ago

Yes - that's what @dee-ynput proposed, but that wouldn't technically allow it to be overridden per user, right? Say, one dev prefers a different terminal.

the same story as with any other artist. One prefers Blender the other Maya for modelling. What is the difference?

MustafaJafar commented 3 months ago

Yes - that's what @dee-ynput proposed, but that wouldn't technically allow it to be overridden per user, right? Say, one dev prefers a different terminal.

the same story as with any other artist. One prefers Blender the other Maya for modelling. What is the difference?

powershell can be tricky, because, I had to download a new version of it. and I'm not sure if users will keep the default installation path.

BigRoy commented 3 months ago

Yes - that's what @dee-ynput proposed, but that wouldn't technically allow it to be overridden per user, right? Say, one dev prefers a different terminal.

the same story as with any other artist. One prefers Blender the other Maya for modelling. What is the difference?

Thanks @m-u-r-p-h-y this should now work - and user should be able to pick the relevant configured shell application if there's more than one.

BigRoy commented 3 months ago

For whatever reason the Application icon doesn't work for me in the launcher. Does it need to be in public/ or in client/ayon_applications/icons, or both? @iLLiCiTiT

BigRoy commented 2 months ago

Funny thing, When using the Shell main, doesn't open in a new tab. Maybe we need to add start when calling cmd.exe as mentioned here But, we can leave it for another PR.

Yes correct - the actual shell application launching doesn't work because it doesn't open a new console.

I had to force these creationflags to make it work: https://github.com/ynput/ayon-applications/blob/2d58dcd1611ba35ad089f28cd40717f9f8f4bd73/client/ayon_applications/plugins/launcher_actions/debug_shell.py#L176 for what I'm using to run the shell.

BigRoy commented 2 months ago

Thanks @Lypsolon

Could you confirm a few things?

Houdini 19.5 is configured and enabled in the Project's anatomy. That is why it is listed. Unlike the default applications behavior it does not hide applications you do not have installed but are enabled/configured in your project. I am pretty sure that is your case.

We can add functionality that makes it behave similar to other apps so that it only lists any applications that exist if that settings is enabled in Application settings. (It's at the far bottom of the application studio settings I believe)

Then two, the shell not showing may be equivalent to why it failed initially on Windows, on Windows opening a shell via subprocess required us to disconnect stdin, stdout and launch with subprocess CREATE_NEW_CONSOLE flag which is not available under Linux. So what would be interesting to test is whether subprocess.Popen("/usr/bin/zsh") works for you or whether you also need to pass additional arguments for it to correctly open a new console for the command prompt. What works for you?

Lypsolon commented 2 months ago

Houdini 19.5 is configured and enabled in the Project's anatomy. That is why it is listed. Unlike the default applications behavior it does not hide applications you do not have installed but are enabled/configured in your project. I am pretty sure that is your case.

it looks like it, tbh.

We can add functionality that makes it behave similar to other apps so that it only lists any applications that exist if that settings is enabled in Application settings. (It's at the far bottom of the application studio settings I believe)

I think that might be good to do as this behaviour could be confusing. at the same time I think we should put it on the bottom of the list.

Then two, the shell not showing may be equivalent to why it failed initially on Windows, on Windows opening a shell via subprocess required us to disconnect stdin, stdout and launch with subprocess CREATE_NEW_CONSOLE flag which is not available under Linux. So what would be interesting to test is whether subprocess.Popen("/usr/bin/zsh") works for you or whether you also need to pass additional arguments for it to correctly open a new console for the command prompt. What works for you?

ähh no, it doesn't because the command will open a shell, not a terminal window. So, at the end of the day, yes, it will open a new shell inside the current shell, but no window will pop up. To open a terminal window, call the associated system from the desktop/window manager. in gnome that would be subprocess.run(["gnome-terminal"]) PS: there might be other ways to do the same, but I only know this one.

MustafaJafar commented 2 months ago

@Lypsolon does the debug shell launcher work as expected but it doesn't open a new window ? or the shell app is the one that doesn't open in a new window ?

I'm just trying to figure out what is still missing ?

BigRoy commented 2 months ago

@Lypsolon does the debug shell launcher work as expected but it doesn't open a new window ? or the shell app is the one that doesn't open in a new window ?

I'm just trying to figure out what is still missing ?

I suppose both won't work since the issue is the same, we need a way to spawn of a new console for the shell. We just need to find the canonical way to open a new terminal window on Linux similar to the command on Windows, hopefully one that doesn't have to be gnome specific. Or we just tell Linux admins to set up the executable paths to include the gnome-terminal command.

Also assume that Lyon never added the Shell app to the project so didn't use just the shell (without any app environment other than its own).

Lypsolon commented 2 months ago

@Lypsolon does the debug shell launcher work as expected but it doesn't open a new window ? or the shell app is the one that doesn't open in a new window ? I'm just trying to figure out what is still missing ?

I suppose both won't work since the issue is the same, we need a way to spawn of a new console for the shell. We just need to find the canonical way to open a new terminal window on Linux similar to the command on Windows, hopefully one that doesn't have to be gnome specific. Or we just tell Linux admins to set up the executable paths to include the gnome-terminal command.

Also assume that Lyon never added the Shell app to the project so didn't use just the shell (without any app environment other than its own).

I tested adding it to the project, but to no surprise, I got the same result.

# A list of terminal emulators that might be installed.
# This allows admins to add extras, and we could cover a few defaults.
# This also allows the addition of more defaults as requests come in. 
terminal_emulators_addon_settings = [
    "gnome-terminal",
    "x-terminal-emulator",
    "konsole",
    "xfce4-terminal",
    "lxterminal",
    "mate-terminal",
    "tilix",
    "deepin-terminal",
    "terminator",
    "alacritty",
    "st",
    "urxvt",
    "xterm"
]

def get_terminal():
    for terminal in terminal_emulators:
        if shutil.which(terminal):
            return terminal
    return None

subprocess.run([get_terminal()])

MustafaJafar commented 2 months ago

A fun fact, I thought this PR was merged already :D Anyways, should we merge it and fix the issue later ?

The issue : quoted from https://github.com/ynput/ayon-applications/pull/12#issuecomment-2271166843

find the canonical way to open a new terminal window on Linux similar to the command on Windows,
hopefully one that doesn't have to be gnome specific. 
Or we just tell Linux admins to set up the executable paths to include the gnome-terminal command.

Lypsolon commented 2 months ago

A fun fact, I thought this PR was merged already :D Anyways, should we merge it and fix the issue later ?

The issue : quoted from #12 (comment)
find the canonical way to open a new terminal window on Linux similar to the command on Windows,
hopefully one that doesn't have to be gnome specific. 
Or we just tell Linux admins to set up the executable paths to include the gnome-terminal command.

correct me if I'm wrong, but currently, we do not support Linux at all, so if we want to merge it, I believe we should implement some solution or mark the feature as Windows only.

MustafaJafar commented 2 months ago

correct me if I'm wrong, but currently, we do not support Linux at all, so if we want to merge it, I believe we should implement some solution or mark the feature as Windows only.

As far as I know, this feature works pretty well on windows. about the Linux support, I'm not sure about its status, so I'll leave it for @iLLiCiTiT Also, tagging @m-u-r-p-h-y could you tell us if marking this as windows only satisfies the client request ? Tagging @dee-ynput

iLLiCiTiT commented 2 months ago

There is conversation in the code. I would like to remove terminal from settings https://github.com/ynput/ayon-applications/pull/12#discussion_r1714905194 .

dee-ynput commented 2 months ago

My 2cents:

About having a "Shell" in application

It's totally relevant. And for those who don't use it, it's like having blender there even though you don't use it. I do see how problematic it is, but that's true for every entry so let's not make this one any special. We need it in order to have a official shell app which is used by other things (like the open this app in a shell action, but more will come).

Yes, it's a developer tool. But AYON is made for developers too.

Let's add it.

About configuring a shell per user/site

We don't need that. There would be one or two, probably never more than 3 supported in a studio. Let's make them variant of the application. If a project wants to add more, additionnal app would help. But we'll deal with this request IF/WHEN it comes from users.

About using the default shell vs an App defined it settings:

I don't see the usecase where would TD set "studio" terminal and it would be useful.

Here is the use case: When you're a TD debugin someone at his station you need to access a standardized shell to start digging around. Artist won't setup their shell env, and even for the ones doing it you don't want to deal with their custom config: you need an official one.

About using wt/powershell/$SHELL/Terminal.app/etc...

Those are good defaults. But all platforms / distribution have alt consoles that a studio may want to use. Settings should give freedom to configure that, even if 90% of the time it will stay on default.

iLLiCiTiT commented 2 months ago

Ok, so I lost my battle, I hope I won't have to say "I told you" in future.

Now let's do the real review...

iLLiCiTiT commented 2 months ago

Can we rename all the shell to terminal @BigRoy ? All platforms are calling it terminal (even windows).

BigRoy commented 1 month ago

@Lypsolon I've tried to change the defaults for the terminal on Linux, to instead use the list you had provided. Likely still won't work, but if there's any chance you could try with new ayon_applications build package and trying to run this... that'd be lovely! ❤️

Also, @iLLiCiTiT will bitch to me about this insane list. And @dee-ynput I suppose too. So feel free to fight about it - just wanted to throw this in there for @Lypsolon to double check whether this would actually be a suitable workflow for Linux, potentially. (Which I suppose @dee-ynput can test nowadays too!?)

iLLiCiTiT commented 1 month ago

Missing icon and label in client/ayon_applications/contants.py.

iLLiCiTiT commented 1 month ago

Also, @iLLiCiTiT will bitch to me about this insane list.

This is what I expected in the automated logic, so fine for me.

BigRoy commented 1 month ago

Missing icon and label in client/ayon_applications/contants.py.

Sorry, where? Not here?

iLLiCiTiT commented 1 month ago

Sorry, where? Not here?

Sorry I'm on different branch, in develop it is in client/ayon_applications/defs.py.

BigRoy commented 1 month ago

Sorry I'm on different branch, in develop it is in client/ayon_applications/defs.py.

Thanks, implemented with https://github.com/ynput/ayon-applications/pull/12/commits/00fe41ca9cb4eb0401fb8a7064f6663025b7ff04

BigRoy commented 1 month ago

If we really want to do this, then code is ok for me.

🙈 🙉 🙊 - Thanks! Time will tell - and potentially hear us crying at night if we were wrong about this. But thanks again for pointing out your standpoint on this with the reviews, appreciate it!

@Lypsolon @dee-ynput ball is in your court to test the Linux side of things.

Lypsolon commented 1 month ago

If we really want to do this, then code is ok for me.

🙈 🙉 🙊 - Thanks! Time will tell - and potentially hear us crying at night if we were wrong about this. But thanks again for pointing out your standpoint on this with the reviews, appreciate it!

@Lypsolon @dee-ynput ball is in your court to test the Linux side of things.

i tried to test package creation and that works but after uploading the server restarts and dose not show the addon.

BigRoy commented 1 month ago

i tried to test package creation and that works but after uploading the server restarts and dose not show the addon.

Thanks. Checking..

@Lypsolon it works fine for me?

ynput / ayon-applications