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

Add ARCH_IS_WSL to detect Windows Subsystem for Linux #4249

Merged
merged 2 commits into from
Feb 17, 2021

Conversation

ChrisJefferson
Copy link
Contributor

This lets us detect the "Windows Subsystem for Linux".

I realise in some ways the naming has gotten weird now, we call Cygwin (one linux in windows) "ARCH_IS_WINDOWS", which WSL is not. While that is weird, I think it makes sense, WSL is a complete install of Linux (usually Ubuntu).

The only time we need ARCH_IS_WSL is when we want to open GUI programs -- I've added help viewers (unfortunately it doesn't seem possible to open either anchors in HTML, which makes this much less useful. I spent some time searching the internet, but everything people say seems to not work in practice, except making a new webpage which just forwards to the anchor, and I didn't want to do that as it would create lots of new temp files/directories).

I plan on using this in the 'profiling' package too, to automatically open HTML pages (I originally wrote this in that package, then thought I might be generally useful).

@ChrisJefferson ChrisJefferson added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 15, 2021
Copy link
Member

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

Nice. I've got no suggestions on the code but I do have a cosmetic nitpick on the doc.

lib/system.g Outdated Show resolved Hide resolved
lib/helpview.gi Show resolved Hide resolved
@fingolfin
Copy link
Member

Just wondering, how is this related to PR #3939?

@ChrisJefferson
Copy link
Contributor Author

This is unrelated to cygwin changes (although in the long term, the plan is the two distros will come closer together. This will be one place where they differ, as they have different ways of spawning files in windows)

@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #4249 (56a593e) into master (ce4b50f) will decrease coverage by 0.35%.
The diff coverage is 61.76%.

@@            Coverage Diff             @@
##           master    #4249      +/-   ##
==========================================
- Coverage   82.26%   81.90%   -0.36%     
==========================================
  Files         686      682       -4     
  Lines      296672   294708    -1964     
==========================================
- Hits       244045   241393    -2652     
- Misses      52627    53315     +688     
Impacted Files Coverage Δ
lib/mapping.gd 100.00% <ø> (ø)
lib/helpview.gi 27.15% <51.85%> (-10.79%) ⬇️
lib/system.g 76.26% <100.00%> (+0.28%) ⬆️
src/gap.c 73.47% <100.00%> (-3.24%) ⬇️
src/sysfiles.c 40.22% <100.00%> (-0.82%) ⬇️
src/libgap-api.c 3.50% <0.00%> (-60.50%) ⬇️
src/sysjmp.c 52.17% <0.00%> (-30.44%) ⬇️
src/compiler.c 67.53% <0.00%> (-21.42%) ⬇️
src/error.c 73.87% <0.00%> (-15.46%) ⬇️
src/vars.c 82.29% <0.00%> (-11.86%) ⬇️
... and 86 more

@ChrisJefferson
Copy link
Contributor Author

Hopefully all cleaned up now. I decided not to fix other true, I thought they could be sweeped (also false) in another PR.

@wilfwilson
Copy link
Member

Sure, thanks Chris. I'll merge this when the tests pass.

@wilfwilson wilfwilson added the gapdays2021-spring Issues and PRs that could be tackled or discussed at https://www.gapdays.de/gapdays2021-spring label Feb 17, 2021
@fingolfin fingolfin merged commit a18deda into gap-system:master Feb 17, 2021
@ChrisJefferson ChrisJefferson deleted the arch_is_wsl branch April 1, 2022 08:17
@fingolfin fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 17, 2022
@fingolfin fingolfin changed the title Add ARCH_IS_WSL, to detect Windows Subsystem for Linux Add ARCH_IS_WSL to detect Windows Subsystem for Linux Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gapdays2021-spring Issues and PRs that could be tackled or discussed at https://www.gapdays.de/gapdays2021-spring kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements kind: new feature release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants