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

[corlib] Import FileSystemInfo and family from CoreFX #11342

Merged
merged 7 commits into from
Dec 7, 2018
Merged

[corlib] Import FileSystemInfo and family from CoreFX #11342

merged 7 commits into from
Dec 7, 2018

Conversation

alexischr
Copy link
Contributor

@alexischr alexischr commented Oct 24, 2018 •
edited
Loading

Includes the following from System.IO:

  • FileSystemInfo
  • FileInfo
  • DirectoryInfo
  • Directory
  • FileSystem
  • Path Join() Span API

and all of System.IO.Enumeration namespace.

Adds about 1600 System.IO tests

Behavior changes:

  • File.Replace now replaces files even if they are read-only (using the rename syscall on Unix)
  • Directory enumeration is no longer stable in the way that many Mono tests expect
  • Some changes in when/whether some representations of directories contain a trailing separator character
  • Changes in which exceptions are thrown (sometimes these are platform-dependent on CoreFX)

Issues:

Part of #7246

@alexischr alexischr force-pushed the corefx-System.IO-trivial-types branch 5 times, most recently from 4116ff9 to 9f258cd Compare October 24, 2018 15:30
@alexischr
Copy link
Contributor Author

@monojenkins build

1 similar comment
@alexischr
Copy link
Contributor Author

@monojenkins build

@alexischr alexischr force-pushed the corefx-System.IO-trivial-types branch from 28aa48c to fdd9403 Compare October 24, 2018 22:25
@alexischr alexischr force-pushed the corefx-System.IO-trivial-types branch 6 times, most recently from 69de4f7 to 0dc3992 Compare November 1, 2018 02:21
@luhenry luhenry removed their request for review November 6, 2018 16:15
@alexischr alexischr mentioned this pull request Nov 14, 2018
Merged
@alexischr alexischr force-pushed the corefx-System.IO-trivial-types branch 5 times, most recently from cd003c2 to 7f22c28 Compare November 15, 2018 19:54
marek-safar
marek-safar previously requested changes Nov 17, 2018
View reviewed changes
mcs/class/corlib/corefx/MonoLinqHelper.cs Outdated

namespace System.IO
{
internal static class MonoLinqHelper {
Copy link
Member

Choose a reason for hiding this comment

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

You don't need this we already have EnumerableHelpers

mcs/class/corlib/Test/System.IO/DirectoryTest.cs
@@ -1113,7 +1109,7 @@ public void SetLastWriteTimeException2 ()
}

[Test]
[ExpectedException(typeof(FileNotFoundException))]
[ExpectedException()]
Copy link
Member

Choose a reason for hiding this comment

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

Why was the type removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These now throw DirectoryNotFoundException on Unix, and FileNotFoundException on Windows

mcs/class/corlib/System.IO/FileSystem.NotSupported.cs Outdated Show resolved Hide resolved
mcs/class/corlib/System.IO.Enumeration/FileSystemEnumerator.NotSupported.cs Show resolved Hide resolved
@alexischr
[corlib] Import FileSystemInfo and family from CoreFX
8bfc22f
@alexischr
Needed for mono/corefx@7a2894c
7bb8b7e
@alexischr
[acceptance-tests] Disable marshalgetlastwin32error.cs
0d16c27 
The test that runs on Mono (004.1) claims to assert that SetLastError works for pInvokes, but fails to use the Flag when declaring the used pInvoke so it was testing that the last errno happened to be zero.

If fixed to use the flag, the test fails as the errno after getenv() is 13; this may be a Mono bug or something else broken with the test
@alexischr alexischr force-pushed the corefx-System.IO-trivial-types branch from d70b8a8 to 0d16c27 Compare December 5, 2018 03:59
@alexischr alexischr force-pushed the corefx-System.IO-trivial-types branch from 091f705 to 73466e7 Compare December 5, 2018 08:44
@alexischr
Copy link
Contributor Author

@monojenkins commit csproj

@alexischr alexischr dismissed marek-safar’s stale review December 5, 2018 22:26

Issues addressed and all checks except apidiff should pass, please review :)

@alexischr alexischr force-pushed the corefx-System.IO-trivial-types branch from 5b0b91e to 83be2a3 Compare December 6, 2018 00:38
@alexischr
Copy link
Contributor Author

@monojenkins build Linux AArch64 Coop Suspend

@alexischr
Copy link
Contributor Author

@monojenkins backport to 2018-12

@monojenkins monojenkins mentioned this pull request Dec 6, 2018
Closed
@alexischr
Copy link
Contributor Author

@marek-safar can you approve/merge?

@marek-safar
Copy link
Member

@monojenkins commit apidiff

monojenkins added a commit to mono/api-snapshot that referenced this pull request Dec 7, 2018
@monojenkins
Apply changes from mono/mono#11342 to API snapshot
657b2aa
@marek-safar marek-safar merged commit 449e366 into mono:master Dec 7, 2018
@NattyNarwhal NattyNarwhal mentioned this pull request Dec 8, 2018
Closed
@jeromelaban jeromelaban mentioned this pull request Dec 19, 2018
Closed
@akoeplinger akoeplinger mentioned this pull request Feb 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marek-safar marek-safar marek-safar approved these changes

@akoeplinger akoeplinger Awaiting requested review from akoeplinger

@baulig baulig Awaiting requested review from baulig

@alexrp alexrp Awaiting requested review from alexrp

@lateralusX lateralusX Awaiting requested review from lateralusX

@EgorBo EgorBo Awaiting requested review from EgorBo

@lewurm lewurm Awaiting requested review from lewurm

@vargaz vargaz Awaiting requested review from vargaz

@directhex directhex Awaiting requested review from directhex

@kg kg Awaiting requested review from kg

@BrzVlad BrzVlad Awaiting requested review from BrzVlad

@jaykrell jaykrell Awaiting requested review from jaykrell

@jonpryor jonpryor Awaiting requested review from jonpryor

@kjpou1 kjpou1 Awaiting requested review from kjpou1

@lambdageek lambdageek Awaiting requested review from lambdageek

@rolfbjarne rolfbjarne Awaiting requested review from rolfbjarne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alexischr @marek-safar @monojenkins