Skip to content

[runtime] Clarify redirect_io behaviour - #7409

Merged
spouliot merged 1 commit into
dotnet:masterfrom
spouliot:redirect-std
Nov 12, 2019
Merged

[runtime] Clarify redirect_io behaviour#7409
spouliot merged 1 commit into
dotnet:masterfrom
spouliot:redirect-std

Conversation

@spouliot

Copy link
Copy Markdown
Contributor

Change redirect_io to be easier to re-use correctly, i.e. returning the
file descriptor (instead of 0).

Comment why the file descriptor are never released. It makes sense but
it's not obvious (even less in redirect_io) without comments.

Using static fields also makes it easier for static analysis (locals are
easier to process).

Finally reuse, mostly, the same string for the error message, i.e.

$ strings runtime//.libs/mac/libxammac.x86_64.dylib | grep "Could not redirect"
Xamarin.Mac: Could not redirect %s to `%s': %s

versus

$ strings runtime//.libs/mac/libxammac.x86_64.dylib | grep "Could not redirect"
Xamarin.Mac: Could not redirect stdout to `%s': %s
Xamarin.Mac: Could not redirect stderr to `%s': %s

Change `redirect_io` to be easier to re-use correctly, i.e. returning the
file descriptor (instead of `0`).

Comment why the file descriptor are never released. It makes sense but
it's not obvious (even less in `redirect_io`) without comments.

Using static fields also makes it easier for static analysis (locals are
easier to process).

Finally reuse, mostly, the same string for the error message, i.e.

```
$ strings runtime//.libs/mac/libxammac.x86_64.dylib | grep "Could not redirect"
Xamarin.Mac: Could not redirect %s to `%s': %s
```
versus
```
$ strings runtime//.libs/mac/libxammac.x86_64.dylib | grep "Could not redirect"
Xamarin.Mac: Could not redirect stdout to `%s': %s
Xamarin.Mac: Could not redirect stderr to `%s': %s
```
@spouliot
spouliot requested a review from rolfbjarne as a code owner November 12, 2019 08:04
@monojenkins

Copy link
Copy Markdown
Contributor

Build failure
Build failed or was aborted

🔥 Build failed 🔥

@spouliot

Copy link
Copy Markdown
Contributor Author

build

@monojenkins

Copy link
Copy Markdown
Contributor

Build failure
Build succeeded
API Diff (from stable)
API Diff (from PR only) (no change)
Generator Diff (no change)
🔥 Test run failed 🔥

Test results

20 tests failed, 67 tests passed.

Failed tests

  • link all/Mac Modern/Debug: Failed (Test run failed.)
  • link all/Mac Modern/Release: Failed (Test run failed.)
  • link sdk/Mac Modern/Debug: Failed (Test run failed.)
  • link sdk/Mac Modern/Release: Failed (Test run failed.)
  • monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
  • framework-test/watchOS 32-bits - simulator/Debug: Crashed
  • interdependent-binding-projects/watchOS 32-bits - simulator/Debug: Crashed
  • fsharp/watchOS 32-bits - simulator/Debug: Crashed
  • introspection/watchOS 32-bits - simulator/Debug: Crashed
  • dont link/watchOS 32-bits - simulator/Debug: Crashed
  • dont link/watchOS 32-bits - simulator/Release: Crashed
  • link all/watchOS 32-bits - simulator/Debug: Crashed
  • link all/watchOS 32-bits - simulator/Release: Crashed
  • link sdk/watchOS 32-bits - simulator/Debug: Crashed
  • link sdk/watchOS 32-bits - simulator/Release: Crashed
  • mono-native-compat/watchOS 32-bits - simulator/Debug: Crashed
  • mono-native-unified/watchOS 32-bits - simulator/Debug: Crashed
  • monotouch-test/watchOS 32-bits - simulator/Debug (LinkSdk): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Debug (static registrar): Crashed
  • monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): Crashed

@spouliot

Copy link
Copy Markdown
Contributor Author

many, many HE0038 w/watchOS -> https://github.com/xamarin/maccore/issues/581
a few HTTP 500 errors -> fixed with inflight #7401

@spouliot
spouliot merged commit 0b323ca into dotnet:master Nov 12, 2019
@spouliot
spouliot deleted the redirect-std branch November 12, 2019 11:34
@spouliot spouliot added the not-notes-worthy Ignore for release notes label Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

not-notes-worthy Ignore for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants