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

Let user specify existing PREFIX/RISCV for toolchain installs #334

Merged
merged 1 commit into from
Dec 13, 2019

Conversation

jerryz123
Copy link
Contributor

@jerryz123 jerryz123 commented Nov 12, 2019

I like to manage my $RISCV independently.

@jerryz123 jerryz123 requested a review from alonamid November 12, 2019 09:19
@jerryz123 jerryz123 requested a review from a0u November 12, 2019 22:45
@alonamid
Copy link
Contributor

In this case, if you've installed riscv-tools, and now decide you want to install esp-tools, you need to actively clean your RISC-V environment variable (otherwise it will silently now install esp-tools). Is there a way to indicate this to the unsuspecting user?

@jerryz123
Copy link
Contributor Author

In this case, if you've installed riscv-tools, and now decide you want to install esp-tools, you need to actively clean your RISC-V environment variable (otherwise it will silently now install esp-tools). Is there a way to indicate this to the unsuspecting user?

Why would I need to actively clean my RISCV directory? The script should just overwrite existing files in there when it builds esp-tools.

@a0u
Copy link
Member

a0u commented Nov 13, 2019

What @alonamid probably meant was, if one intends to build an alternative toolchain but has already sourced env.sh, the user must remember to unset RISCV to avoid overwriting the original installation.

I am fine with supporting arbitrary installation paths, but an explicit command line option (-d <path>) would be preferable instead. Regardless of the approach, the user-specified input should also be converted into an absolute path, if not already one, to avoid a subsequent configure error.

@sagark
Copy link
Member

sagark commented Nov 13, 2019

+1 to @a0u

@jerryz123 jerryz123 changed the title Do not override existing RISCV env-variable Let user specify existing PREFIX/RISCV for toolchain installs Dec 2, 2019
@jerryz123
Copy link
Contributor Author

I added a --prefix option. I also incremented the CI cached tools index, so the build-tools tests reran.

Copy link
Contributor

@alonamid alonamid left a comment

Choose a reason for hiding this comment

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

LGTM

@jerryz123 jerryz123 merged commit 72f9730 into dev Dec 13, 2019
@jerryz123 jerryz123 deleted the build-script-patch branch January 22, 2020 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants