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

Improve Mason error when git is missing #25069

Merged
merged 8 commits into from
May 21, 2024

Conversation

jabraham17
Copy link
Member

@jabraham17 jabraham17 commented May 16, 2024

Improves the error thrown by Mason when git is missing. Prior to this PR, all that would be thrown was "Internal mason error", which is not helpful.

PR also includes a bonus typo fix in the docs for Subprocess

Checked that the test for this error message passes

[Reviewed by @benharsh]

Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
@benharsh benharsh self-requested a review May 20, 2024 22:53
@benharsh
Copy link
Member

Do we not have a way of detecting that git is totally unavailable, such that we can say that directly? To me, these messages suggest that the sub-command of git failed, rather than git not existing.

@jabraham17
Copy link
Member Author

Do we not have a way of detecting that git is totally unavailable, such that we can say that directly?

The best way in Chapel code I could come up with was running a loop over all directories in PATH (as returned by getenv) and see if git exists in any of them. This felt overly heavy handed to me.

Another slightly cleaner alternative is to have spawn(["which", "git"]), but that's less portable as not all systems have which installed by default

Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
Signed-off-by: Jade Abraham <jade.abraham@hpe.com>
@jabraham17 jabraham17 requested a review from benharsh May 21, 2024 18:59
@jabraham17 jabraham17 merged commit 2cecbad into chapel-lang:main May 21, 2024
7 checks passed
@jabraham17 jabraham17 deleted the mason-improve-error branch May 21, 2024 21:41
jabraham17 added a commit that referenced this pull request May 22, 2024
This PR expands on the work in
#25069 to improve mason errors
when dependencies (specifically git) are missing. This PR adds
additional logic to account for portability issues exposed by nightly
testing

Testing
- Tested the two tests added in #25069 pass on a Mac
- Tested `start_test test/mason` passes on linux x86
- Tested `start_test test/mason` passes on the nightly test machine (a
slightly different linux x86)

[Reviewed by @benharsh]
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.

None yet

2 participants