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

[v4] Add installed tools to path #180

Merged
merged 8 commits into from
Oct 20, 2023

Conversation

ddalcino
Copy link
Contributor

Fix #157

As discussed in #157, the preferred way to handle this is to append tool paths to the PATH, so that the added paths don't unintentionally override other tools that you're trying to keep in your environment. The tools provided by aqtinstall tend to include a lot of binaries that you may or may not actually need.

Unfortunately, every attempt I have made to append tool paths has failed miserably on Windows. Github toolkit/core provides a convenient and robust method of prepending to the path, and that's what this PR uses, because it works really well. Unfortunately, Github did not provide an equivalent "append to path" method that works on Windows, and nothing I have tried has worked so far.

I am submitting this as a draft because it prepends to the path, rather than appends. Hopefully this will help generate ideas and discussion as to how to move forward.

@ddalcino ddalcino changed the title Add installed tools to path [v4] Add installed tools to path May 9, 2023
@ddalcino
Copy link
Contributor Author

ddalcino commented May 9, 2023

Since this PR would make interface-breaking changes that could easily require users to rewrite their workflows, this PR should not be merged into v3 of the action. This change would need to go into the next major version.

@ddalcino ddalcino marked this pull request as ready for review May 9, 2023 03:30
@jurplel
Copy link
Owner

jurplel commented May 13, 2023

Might it be reasonable to make this functionality optional (on by default), in case people have PATH issues?

@ddalcino ddalcino force-pushed the topic/add-tools-to-path2 branch from 187eddf to 9392dfe Compare May 13, 2023 15:56
@ddalcino
Copy link
Contributor Author

Might it be reasonable to make this functionality optional (on by default), in case people have PATH issues?

Yes, that's probably wise.

@ddalcino ddalcino force-pushed the topic/add-tools-to-path2 branch from 974fc62 to 41b28c6 Compare May 16, 2023 02:23
@ddalcino
Copy link
Contributor Author

Added parameter add-tools-to-path, default true, with passing test. Link to run where the test failed, before I added a passing implementation, is here: https://github.com/jurplel/install-qt-action/actions/runs/4987188695/jobs/8928708750

@jurplel jurplel merged commit cb9bd94 into jurplel:master Oct 20, 2023
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.

Add the tool's specific bin path when installing tools
2 participants