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

Console.Unix: revert SetWindowSize implementation. #100272

Merged
merged 8 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
50 changes: 12 additions & 38 deletions src/libraries/System.Console/src/System/Console.cs
Original file line number Diff line number Diff line change
Expand Up @@ -392,42 +392,27 @@ 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; }
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
}
[SupportedOSPlatform("windows")]
set { ConsolePal.WindowWidth = value; }
}

[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 +423,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
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
8 changes: 8 additions & 0 deletions src/libraries/System.Console/src/System/ConsolePal.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1012,6 +1012,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
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
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
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/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.

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
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,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
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
16 changes: 0 additions & 16 deletions src/native/libs/System.Native/pal_console.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,6 @@ int32_t SystemNative_GetWindowSize(intptr_t fd, WinSize* windowSize)
#endif
}

int32_t SystemNative_SetWindowSize(WinSize* windowSize)
{
assert(windowSize != NULL);

#if HAVE_IOCTL_WITH_INT_REQUEST && HAVE_TIOCSWINSZ
return ioctl(STDOUT_FILENO, (int)TIOCSWINSZ, windowSize);
#elif HAVE_IOCTL && HAVE_TIOCSWINSZ
return ioctl(STDOUT_FILENO, TIOCSWINSZ, windowSize);
#else
// Not supported on e.g. Android. Also, prevent a compiler error because windowSize is unused
(void)windowSize;
errno = ENOTSUP;
return -1;
#endif
}

int32_t SystemNative_IsATty(intptr_t fd)
{
return isatty(ToFileDescriptor(fd));
Expand Down
7 changes: 0 additions & 7 deletions src/native/libs/System.Native/pal_console.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,6 @@ typedef struct
*/
PALEXPORT int32_t SystemNative_GetWindowSize(intptr_t fd, WinSize* windowsSize);

/**
* Sets the windows size of the terminal
*
* Returns 0 on success; otherwise, returns -1 and sets errno.
*/
PALEXPORT int32_t SystemNative_SetWindowSize(WinSize* windowsSize);

/**
* Gets whether the specified file descriptor is for a terminal.
*
Expand Down
Loading
Loading