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.

Sign up for GitHub

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

Jump to bottom

Console.Unix: revert SetWindowSize implementation. #100272

Merged
merged 8 commits into from
Aug 14, 2024
Merged

Console.Unix: revert SetWindowSize implementation. #100272

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ff9fe1c
Console.Unix: revert SetWindowSize implementation.
tmds Mar 26, 2024
091ceca
Delete HAVE_TIOCSWINSZ.
tmds Mar 26, 2024
059f2c4
Update compat supressions.
tmds Mar 26, 2024
dca62ef
Merge branch 'main' into console_set_windowsize_pnse
tmds Apr 16, 2024
90fe4aa
Code formatting.
tmds May 16, 2024
26ceecf
Merge branch 'main' into console_set_windowsize_pnse
tmds Jul 2, 2024
43a7290
Apply suggestions from code review
jozkee Aug 13, 2024
3deb479
Merge branch 'main' into console_set_windowsize_pnse
jozkee Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetWindowWidth.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,5 @@ internal struct WinSize

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetWindowSize", SetLastError = true)]
internal static partial int GetWindowSize(SafeFileHandle terminalHandle, out WinSize winSize);

[LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_SetWindowSize", SetLastError = true)]
internal static partial int SetWindowSize(in WinSize winSize);
}
}
17 changes: 3 additions & 14 deletions src/libraries/System.Console/ref/System.Console.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,10 @@ public static partial class Console
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
public static bool TreatControlCAsInput { get { throw null; } set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
public static int WindowHeight { get { throw null; } set { } }
public static int WindowHeight { [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
public static int WindowLeft { get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
public static int WindowTop { get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
public static int WindowWidth { get { throw null; } set { } }
public static int WindowWidth { [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios"), System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] get { throw null; } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] set { } }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
Expand Down Expand Up @@ -155,10 +147,7 @@ public static void SetIn(System.IO.TextReader newIn) { }
public static void SetOut(System.IO.TextWriter newOut) { }
[System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
public static void SetWindowPosition(int left, int top) { }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")]
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")]
[System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
public static void SetWindowSize(int width, int height) { }
public static void Write(bool value) { }
public static void Write(char value) { }
Expand Down
49 changes: 12 additions & 37 deletions src/libraries/System.Console/src/System/Console.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,42 +392,28 @@ public static int WindowTop
set { ConsolePal.WindowTop = value; }
}

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
public static int WindowWidth
{
[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
get { return ConsolePal.WindowWidth; }
[SupportedOSPlatform("windows")]
set
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowWidth));

ConsolePal.WindowWidth = value;
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}
{ ConsolePal.WindowWidth = value; }
tmds marked this conversation as resolved.
Show resolved Hide resolved
}

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
public static int WindowHeight
{
[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
get { return ConsolePal.WindowHeight; }
[SupportedOSPlatform("windows")]
set
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowHeight));

ConsolePal.WindowHeight = value;
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}
}
Expand All @@ -438,20 +424,9 @@ public static void SetWindowPosition(int left, int top)
ConsolePal.SetWindowPosition(left, top);
}

[UnsupportedOSPlatform("android")]
[UnsupportedOSPlatform("browser")]
[UnsupportedOSPlatform("ios")]
[UnsupportedOSPlatform("tvos")]
[SupportedOSPlatform("windows")]
public static void SetWindowSize(int width, int height)
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width, nameof(width));
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height, nameof(height));

ConsolePal.SetWindowSize(width, height);
}

Expand Down
23 changes: 5 additions & 18 deletions src/libraries/System.Console/src/System/ConsolePal.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,24 +378,11 @@ private static void GetWindowSize(out int width, out int height)

public static void SetWindowSize(int width, int height)
{
lock (Console.Out)
{
Interop.Sys.WinSize winsize = default;
winsize.Row = (ushort)height;
winsize.Col = (ushort)width;
if (Interop.Sys.SetWindowSize(in winsize) == 0)
{
s_windowWidth = winsize.Col;
s_windowHeight = winsize.Row;
}
else
{
Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
throw errorInfo.Error == Interop.Error.ENOTSUP ?
new PlatformNotSupportedException() :
Interop.GetIOException(errorInfo);
}
}
// note: We can't implement SetWindowSize using TIOCSWINSZ.
// TIOCSWINSZ is meant to inform the kernel of the terminal size.
// The window that shows the terminal doesn't change to match that size.

throw new PlatformNotSupportedException();
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

public static bool CursorVisible
Expand Down
22 changes: 22 additions & 0 deletions src/libraries/System.Console/src/System/ConsolePal.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,13 @@ public static int WindowWidth
}
set
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowWidth));

jozkee marked this conversation as resolved.
Show resolved Hide resolved
SetWindowSize(value, WindowHeight);
}
}
Expand All @@ -931,6 +938,13 @@ public static int WindowHeight
}
set
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowHeight));

jozkee marked this conversation as resolved.
Show resolved Hide resolved
SetWindowSize(WindowWidth, value);
}
}
Expand Down Expand Up @@ -963,6 +977,14 @@ public static unsafe void SetWindowPosition(int left, int top)

public static unsafe void SetWindowSize(int width, int height)
{
if (Console.IsOutputRedirected)
{
throw new IOException(SR.InvalidOperation_SetWindowSize);
}

ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width, nameof(width));
ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height, nameof(height));

// Get the position of the current console window
Interop.Kernel32.CONSOLE_SCREEN_BUFFER_INFO csbi = GetBufferInfo();

Expand Down
33 changes: 0 additions & 33 deletions src/libraries/System.Console/tests/ManualTests/ManualTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
using System.Diagnostics;
using System.Threading.Tasks;
using System.IO;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using Xunit;

namespace System
Expand Down Expand Up @@ -334,37 +332,6 @@ public static void CursorLeftFromLastColumn()
AssertUserExpectedResults("single line with '1' at the start and '2' at the end.");
}

[ConditionalFact(nameof(ManualTestsEnabled))]
[SkipOnPlatform(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Not supported on Browser, iOS, MacCatalyst, or tvOS.")]
public static void ResizeTest()
{
bool wasResized = false;

using (ManualResetEvent manualResetEvent = new(false))
using (PosixSignalRegistration.Create(PosixSignal.SIGWINCH,
ctx =>
{
wasResized = true;
Assert.Equal(PosixSignal.SIGWINCH, ctx.Signal);
manualResetEvent.Set();
}))
{
int widthBefore = Console.WindowWidth;
int heightBefore = Console.WindowHeight;

Assert.False(wasResized);

Console.SetWindowSize(widthBefore / 2, heightBefore / 2);

Assert.True(manualResetEvent.WaitOne(TimeSpan.FromMilliseconds(50)));
Assert.True(wasResized);
Assert.Equal(widthBefore / 2, Console.WindowWidth );
Assert.Equal(heightBefore / 2, Console.WindowHeight );

Console.SetWindowSize(widthBefore, heightBefore);
}
}

private static void AssertUserExpectedResults(string expected)
{
Console.Write($"Did you see {expected}? [y/n] ");
Expand Down
10 changes: 5 additions & 5 deletions src/libraries/System.Console/tests/WindowAndCursorProps.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static void SetBufferSize_Unix_ThrowsPlatformNotSupportedException()
}

[Theory]
[PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
[PlatformSpecific(TestPlatforms.Windows)]
[InlineData(0)]
[InlineData(-1)]
public static void WindowWidth_SetInvalid_ThrowsArgumentOutOfRangeException(int value)
Expand All @@ -71,14 +71,14 @@ public static void WindowWidth_GetUnix_Success()
}

[Fact]
[PlatformSpecific(TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)] // Expected behavior specific to Unix
[PlatformSpecific(TestPlatforms.AnyUnix)]
public static void WindowWidth_SetUnix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.WindowWidth = 100);
}

[Theory]
[PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
[PlatformSpecific(TestPlatforms.Windows)]
[InlineData(0)]
[InlineData(-1)]
public static void WindowHeight_SetInvalid_ThrowsArgumentOutOfRangeException(int value)
Expand Down Expand Up @@ -111,7 +111,7 @@ public static void LargestWindowWidth_UnixGet_ReturnsExpected()
}

[Fact]
[PlatformSpecific(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)] // Expected behavior specific to Unix
[PlatformSpecific(TestPlatforms.AnyUnix)]
public static void WindowHeight_SetUnix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.WindowHeight = 100);
Expand Down Expand Up @@ -579,7 +579,7 @@ public void SetWindowSize_GetWindowSize_ReturnsExpected()
}

[Fact]
[PlatformSpecific(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)]
[PlatformSpecific(TestPlatforms.AnyUnix)]
public void SetWindowSize_Unix_ThrowsPlatformNotSupportedException()
{
Assert.Throws<PlatformNotSupportedException>(() => Console.SetWindowSize(50, 50));
Expand Down
54 changes: 54 additions & 0 deletions src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/en-us/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Console.SetWindowSize(System.Int32,System.Int32):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/mscorlib.dll</Left>
<Right>net9.0/mscorlib.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowHeight:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/mscorlib.dll</Left>
<Right>net9.0/mscorlib.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowWidth:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/mscorlib.dll</Left>
<Right>net9.0/mscorlib.dll</Right>
</Suppression>
Copy link
Member

Choose a reason for hiding this comment

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

How was this generated? I tried as described in #75824 (comment) but didn't get any changes related to Console.
cc @ericstj

Copy link
Member

Choose a reason for hiding this comment

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

@ViktorHofer

Copy link
Member

Choose a reason for hiding this comment

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

dotnet.cmd src/libraries/apicompat --no-dependencies /p:ApiCompatGenerateSuppressionFile=true should work, make sure you've built libs before doing that.

I think the comment you linked to had a different property name. The one listed in the error message and shared here should be the right one: ApiCompatGenerateSuppressionFile
https://learn.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#apicompatgeneratesuppressionfile

Copy link
Member

Choose a reason for hiding this comment

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

Also these are all CP0014 - they are what I'd expect for the attribute changes to the reference assembly. If you're OK with those changes then these are fine (so long as build passes they are in sync).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for checking them out. Btw, this is what I get from running dotnet.cmd:

C:\git\runtime>dotnet.cmd src\libraries\apicompat --no-dependencies /p:ApiCompatGenerateSuppressionFile=true
C:\Program Files\dotnet
Could not execute because the specified command or file was not found.
Possible reasons for this include:
  * You misspelled a built-in dotnet command.
  * You intended to execute a .NET program, but dotnet-src\libraries\apicompat does not exist.
  * You intended to run a global tool, but a dotnet-prefixed executable with this name could not be found on the PATH.

<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down Expand Up @@ -91,6 +109,12 @@
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Console.SetWindowSize(System.Int32,System.Int32):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down Expand Up @@ -121,6 +145,18 @@
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowHeight:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowWidth:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/netstandard.dll</Left>
<Right>net9.0/netstandard.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down Expand Up @@ -349,6 +385,24 @@
<Left>net8.0/System.ComponentModel.TypeConverter.dll</Left>
<Right>net9.0/System.ComponentModel.TypeConverter.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.Console.SetWindowSize(System.Int32,System.Int32):[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/System.Console.dll</Left>
<Right>net9.0/System.Console.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowHeight:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/System.Console.dll</Left>
<Right>net9.0/System.Console.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>P:System.Console.WindowWidth:[T:System.Runtime.Versioning.UnsupportedOSPlatformAttribute]</Target>
<Left>net8.0/System.Console.dll</Left>
<Right>net9.0/System.Console.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0014</DiagnosticId>
<Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
Expand Down
1 change: 0 additions & 1 deletion src/native/libs/Common/pal_config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
#cmakedefine01 HAVE_IOCTL
#cmakedefine01 HAVE_IOCTL_WITH_INT_REQUEST
#cmakedefine01 HAVE_TIOCGWINSZ
#cmakedefine01 HAVE_TIOCSWINSZ
#cmakedefine01 HAVE_SCHED_GETAFFINITY
#cmakedefine01 HAVE_SCHED_SETAFFINITY
#cmakedefine01 HAVE_SCHED_GETCPU
Expand Down
1 change: 0 additions & 1 deletion src/native/libs/System.Native/entrypoints.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ static const Entry s_sysNative[] =
{
DllImportEntry(SystemNative_FStat)
DllImportEntry(SystemNative_GetWindowSize)
DllImportEntry(SystemNative_SetWindowSize)
DllImportEntry(SystemNative_IsATty)
DllImportEntry(SystemNative_InitializeTerminalAndSignalHandling)
DllImportEntry(SystemNative_SetKeypadXmit)
Expand Down
Loading
Loading