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

New approach to setup user-env on UNIX #94

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

edassis
Copy link
Contributor

@edassis edassis commented Jan 30, 2025

An 'env' file is created now, heavily inspired by the rust's cargo approach (See #65), located in '~/.config/godotenv/env'. It's generated by GodotEnv and contains commands in 'sh' syntax to set up the user shell.

The new 'env' file is responsible for:

  • Prepend the GodotEnv bin folder path into PATH;
  • 'Export' GODOT env-var pointing to Godot symlink.

We expect all POSIX compatible shells (or able to interpret 'sh' syntax in some way, like fish shell) to be able to load the new GodotEnv's env file.

An auxiliary file was added to set up the 'fish' shell too.

Note

In the main branch, GodotEnv tryes to configure "PATH" and "GODOT" env-vars using heuristcs to detect the user default shell. But it showed itself cumbersome and only supported bash and zsh shells. The new approach uses sh shell when necessary (for GetUserEnv() basically) and is indiferrent about the user's default shell, what I think it's a big plus.
Captura de tela 2025-01-28 130115
Although, with the changes in this PR, currently GodotEnv users will have PATH and GODOT exports in their shell rc file and those will not be cleaned up automatically. I thought in making a note about it in the release telling people to just removes those lines. We could do some logic trying to detect it and clean them, but for the sake of making the code simple I prefered to not do it (what your thoughts?), and in case the user not removes them it will not affect the GodotEnv's functionality.

Fixes #65

Let me know what you think.


The relevant commit it's the last one (has the same title as this PR), the others commits are from PR #93 waiting for merge. (rebased onto main)

@jolexxa
Copy link
Member

jolexxa commented Jan 31, 2025

If this is based off of #93, can you change the target branch to point into it? Then, when we merge #93, the target branch will auto-change to main. This should make the diff cleaner in the meantime if I am not mistaken.

@edassis edassis changed the base branch from main to fix/godot-sharp January 31, 2025 23:58
@edassis edassis changed the base branch from fix/godot-sharp to main January 31, 2025 23:59
@edassis
Copy link
Contributor Author

edassis commented Feb 1, 2025

I tried to adjust the base branch, but as the fix/msgs_windows its on my fork, I cant reference it in this PR. But, after merging the other PR, I think the diff here will adjust itself too.
image

@jolexxa
Copy link
Member

jolexxa commented Feb 1, 2025

@edassis okay, didn't realize it couldn't be done from a fork. Makes sense. We'll get #93 merged in and then I'll review this.

@jolexxa
Copy link
Member

jolexxa commented Feb 2, 2025

and in case the user not removes them it will not affect the GodotEnv's functionality.

That sounds perfectly fine. Now that #93 is merged, I'll take a closer look at this.

@jolexxa
Copy link
Member

jolexxa commented Feb 2, 2025

@edassis Oh, this might need to be re-based off main since you had updated the other PR since this one was branched off that. Or at least updated however you prefer.

@edassis
Copy link
Contributor Author

edassis commented Feb 2, 2025

Did a rebase onto main. I don't know why the CI is failing for the PR. The one started from my branch seems fine: https://github.com/edassis/GodotEnv/actions/workflows/install_test.yaml

The install test failed at the checksum verification. Could it be a network instability on the Github CI? It's possible to force a rerun?

@edassis edassis marked this pull request as draft February 4, 2025 05:31
@edassis edassis requested a review from jolexxa February 4, 2025 16:07
@edassis edassis force-pushed the fix/shell_path branch 2 times, most recently from 2039213 to aa6e3ee Compare February 5, 2025 19:20
An 'env' file is created now, heavily inspired by the rust's cargo
approach (See chickensoft-games#65), located in '~/.config/godotenv/env'. It's
generated by GodotEnv and contains commands in 'sh' syntax to setup
the user shell.

The new 'env' file is responsible for:
- Prepend the GodotEnv bin folder path into PATH;
- 'Export' GODOT env-var pointing to Godot symlink.

We expect all POSIX compatible shells to work with it.

Fixes chickensoft-games#65
@edassis
Copy link
Contributor Author

edassis commented Feb 5, 2025

As discussed, removed the fish env setup and prepared a message telling the user that maybe he will need to adjust GODOT and PATH manually depending on his shell. The message only shows on UNIX systems:

Windows

Captura de tela 2025-02-05 162554

Linux

Captura de tela 2025-02-05 162122

@edassis edassis marked this pull request as ready for review February 5, 2025 19:31
Copy link
Member

@jolexxa jolexxa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳 ty!

@jolexxa jolexxa merged commit e85dc93 into chickensoft-games:main Feb 6, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set GODOT environment variable in both ".bashrc" and ".bash_profile" shell configuration files on Unix
2 participants