Skip to content

Portability cleanups: no compiler warning if there is no dlopen; no GAP warning if UNIXSelect is missing - #6540

Open
fingolfin wants to merge 2 commits into
masterfrom
mh/portability-cleanups
Open

Portability cleanups: no compiler warning if there is no dlopen; no GAP warning if UNIXSelect is missing#6540
fingolfin wants to merge 2 commits into
masterfrom
mh/portability-cleanups

Conversation

@fingolfin

Copy link
Copy Markdown
Member

Some more portability tweaks that were created as part of the new MingW port (see issue #4157).

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

fingolfin and others added 2 commits August 29, 2026 01:54
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The method can only ever run for pty based iostreams, which require
subprocess support; without it (native Windows) its reference to
UNIXSelect produced a syntax warning at every startup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@fingolfin fingolfin added the os: windows Issues and PRs that are (at least partially) specific to Windows label Aug 29, 2026
@codecov

codecov Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.98%. Comparing base (989cc35) to head (b54c077).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6540      +/-   ##
==========================================
- Coverage   79.00%   78.98%   -0.03%     
==========================================
  Files         684      684              
  Lines      294205   294209       +4     
  Branches     8647     8681      +34     
==========================================
- Hits       232444   232374      -70     
- Misses      59955    60027      +72     
- Partials     1806     1808       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fingolfin fingolfin added topic: kernel topic: library release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels Aug 31, 2026
Comment thread lib/streams.gi
Comment on lines 1462 to +1468
#############################################################################
##
#M ReadAllLine( <iostream>[, <nofail>][, <IsAllLine>] ) . . read whole line
##
# this method serves pty based iostreams, which only exist on systems that
# also have UNIXSelect
if IsBound( UNIXSelect ) then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if IsBound( UNIXSelect ) then
# this method serves pty based iostreams, which only exist on systems that
# also have UNIXSelect
if IsBound( UNIXSelect ) then
#############################################################################
##
#M ReadAllLine( <iostream>[, <nofail>][, <IsAllLine>] ) . . read whole line
##

I might find it more readable like this, but no strong opinion; just wanted to mention it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

os: windows Issues and PRs that are (at least partially) specific to Windows release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: kernel topic: library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants