Implement FileManager.url{s,}(for:…) and NSSearchPathForDirectoriesInDomains. #1456

millenomi · 2018-02-23T21:39:39Z

Implement urls(for:…) as the primitive directory method:

In Darwin, match paths with the Objective-C implementation wherever possible.
On platforms that use FHS/XDG, implement reading user-dirs.* files to determine XDG directories;

This patch also enables the @testable import syntax in TestFoundation in the Debug configuration.

millenomi · 2018-02-27T23:26:40Z

@swift-ci Please test.

pushkarnk · 2018-02-28T07:27:22Z

@millenomi I'm not sure if @testable import Foundation would have the desired effect on Linux.

pushkarnk · 2018-02-28T07:31:09Z

@millenomi Sorry, I noticed your changes to phases.py later. However, there's still a build failure
TestFoundation/TestFileManager.swift:11:22: error: module 'Foundation' was not compiled for testing

millenomi · 2018-02-28T18:02:56Z

Looks like RelWithDebugInfo still builds Debug; I need to conditionalize these changes for Release.

millenomi · 2018-02-28T18:49:16Z

Or, better, see if CI can build for @testable import. Stay tuned…

millenomi · 2018-03-06T23:05:52Z

@swift-ci please test

shahmishal · 2018-03-06T23:50:07Z

@swift-ci please test

millenomi · 2018-03-07T17:59:02Z

@shahmishal The build failed, but the Foundation tests compile and run, thank you!

millenomi · 2018-03-08T00:49:38Z

@swift-ci please test and merge

pvieito · 2018-03-11T15:41:17Z

Foundation/FileManager.swift

+            return [ URL(fileURLWithPath: "/home", isDirectory: true) ]
+
+        case .moviesDirectory:
+            return []


Could we use _XDGUserDirectory.videos.url here?

Good catch.

shahmishal · 2018-03-12T06:52:22Z

We need to update swiftpm to be able to use --debug-foundation flag

/home/buildnode/jenkins/workspace/swift-corelibs-foundation-PR-Linux/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/swift-test: error while loading shared libraries: libswiftSwiftOnoneSupport.so: cannot open shared object file: No such file or directory
--- bootstrap: error: tests failed with exit status 127

millenomi · 2018-04-03T21:49:34Z

I think we fixed that with #1495 — I built and tested locally and it seemed to go through. Trying again.

millenomi · 2018-04-03T21:49:40Z

@swift-ci please test

millenomi · 2018-04-03T22:21:42Z

The test has not passed but it's not related to this patch. A moment.

millenomi · 2018-04-03T22:37:00Z

@swift-ci please test

spevans · 2018-04-03T22:40:22Z

@millenomi Does this PR need a rebase to resolve the conflict?

millenomi · 2018-04-03T22:42:14Z

… oh. Yes, yes it does.

millenomi · 2018-04-03T22:47:32Z

@swift-ci please test

alblue · 2018-04-12T10:39:04Z

@swift-ci please test

pvieito · 2018-05-27T22:42:02Z

What is the status of this PR? I would love to be able to use FileManager.url(for:…) across platforms :)

millenomi · 2018-05-28T07:13:42Z

@swift-ci please test

spevans · 2018-05-28T07:45:21Z

TestFoundation/TestFileManager.swift

@@ -28,7 +28,10 @@ class TestFileManager : XCTestCase {
            ("test_homedirectoryForUser", test_homedirectoryForUser),
            ("test_temporaryDirectoryForUser", test_temporaryDirectoryForUser),
            ("test_creatingDirectoryWithShortIntermediatePath", test_creatingDirectoryWithShortIntermediatePath),
-            ("test_mountedVolumeURLs", test_mountedVolumeURLs),
+            ("test_mountedVolumeURLs", test_mountedVolumeURLs)


I think you need a , at the end of this line

Late night merge mistake. Thanks for the catch.

spevans · 2018-05-28T08:30:46Z

Foundation/FileManager_XDG.swift

+//  Created by Lily Vulcano on 2/26/18.
+//  Copyright © 2018 Apple. All rights reserved.
+//
+


This should probably be the standard swift.org file header

millenomi · 2018-05-28T14:19:07Z

@swift-ci please test.

spevans · 2018-05-28T14:58:18Z

Foundation/FileManager.swift

+                self = .user
+            }
+
+            return nil


Wont this failable initialiser always fail?

How so? It didn’t in local testing. (It will fail for bitmasks with more than one bit set and that’s intended.)

Oh dear. Yeah, I see the problem.

This patch needs some care and attention.

felix91gr · 2018-06-06T00:10:08Z

Thank you for working on this! The FileManager API is possibly the most commonly needed at projects in the community that wish to become cross-platform*. Thank you 🙏

* This includes SPM, which if it had access to the FileManager and FileHandle APIs it uses in Darwin, would be able to synthesize the LinuxMain.swift file (and actually wouldn't really need to create it at all!) in every platform that runs corelibs-foundation.

…ectoriesInDomains. Implement urls(for:…) as the primitive directory method: - In Darwin, match paths with the Objective-C implementation wherever possible. - On platforms that use FHS/XDG, implement reading user-dirs.* files to determine XDG directories; Implement url(for:in:…) and NSSearchPathForDirectoriesInDomains() in terms of urls(for:…).

millenomi · 2018-06-08T22:44:43Z

@swift-ci please test

millenomi · 2018-06-09T16:29:17Z

@swift-ci please test

millenomi · 2018-06-11T21:40:50Z

@swift-ci please test and merge

spevans · 2018-06-12T03:49:35Z

@swift-ci please test and merge

rudkx · 2018-06-12T16:22:17Z

@millenomi It looks like this broke the build on the CI bots, despite having passed PR testing.

https://ci.swift.org/job/oss-swift-incremental-RA-linux-ubuntu-16_04/4410/

rudkx · 2018-06-12T16:35:35Z

This is blocking PR testing as well, so I'm going to open a PR to revert. I don't understand how this passed PR testing unless there was a second PR being tested simultaneously and both are required to cause the failure (or perhaps a clean build was required and it didn't happen for the PR testing)?

millenomi · 2018-06-12T16:53:35Z

On it.

rudkx · 2018-06-12T16:56:05Z

@millenomi Thanks. Happy to hold off on reverting if we can get this resolved pretty quickly. We have another PR (swiftlang/swift#17140) we're trying to get through to fix another breakage on Linux, and I suspect this issue will block merging that PR.

millenomi · 2018-06-12T16:57:01Z

@rudkx I will resolve it today, but it requires some work. Let's just revert for now.

rudkx · 2018-06-12T16:59:38Z

@millenomi Okay, sounds good to me. Thanks!

millenomi requested a review from phausler February 27, 2018 23:26

millenomi changed the title ~~[WIP] Implement FileManager.url{s,}(for:…) and NSSearchPathForDirectoriesInDomains.~~ Implement FileManager.url{s,}(for:…) and NSSearchPathForDirectoriesInDomains. Feb 27, 2018

pvieito reviewed Mar 11, 2018

View reviewed changes

spevans reviewed May 28, 2018

View reviewed changes

millenomi added 2 commits June 8, 2018 09:59

Added tests.

e731323

millenomi added 6 commits June 8, 2018 09:59

Ensure .moviesDirectory maps to XDG’s VIDEOS directory.

939cc36

Fix licensing comment and test syntax from @spevans feedback.

243b353

Merge fixes.

cd7060e

Make this work, and testable under Darwin and Linux.

b935c06

Fix Linux build.

cf348b8

Pass through LD_*/DYLD_* environment variables.

37b8b44

swift-ci merged commit 233ce17 into swiftlang:master Jun 12, 2018

rudkx mentioned this pull request Jun 12, 2018

Revert "Implement FileManager.url{s,}(for:…) and NSSearchPathForDirectoriesInDomains." #1596

Merged

millenomi deleted the XDG branch June 12, 2018 17:48

millenomi mentioned this pull request Jun 12, 2018

[Attempt 2] Implement FileManager.url{s,}(for:…) and NSSearchPathForDirectoriesInDomains. #1597

Merged

spevans mentioned this pull request Jan 16, 2020

[SR-6578] [parity] Implement FileManager search URLs API #3768

Closed

Implement FileManager.url{s,}(for:…) and NSSearchPathForDirectoriesInDomains. #1456

Implement FileManager.url{s,}(for:…) and NSSearchPathForDirectoriesInDomains. #1456

Uh oh!

Conversation

millenomi commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

millenomi commented Feb 27, 2018

Uh oh!

pushkarnk commented Feb 28, 2018

Uh oh!

pushkarnk commented Feb 28, 2018

Uh oh!

millenomi commented Feb 28, 2018

Uh oh!

millenomi commented Feb 28, 2018

Uh oh!

millenomi commented Mar 6, 2018

Uh oh!

shahmishal commented Mar 6, 2018

Uh oh!

millenomi commented Mar 7, 2018

Uh oh!

millenomi commented Mar 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shahmishal commented Mar 12, 2018

Uh oh!

millenomi commented Apr 3, 2018

Uh oh!

millenomi commented Apr 3, 2018

Uh oh!

millenomi commented Apr 3, 2018

Uh oh!

millenomi commented Apr 3, 2018

Uh oh!

spevans commented Apr 3, 2018

Uh oh!

millenomi commented Apr 3, 2018

Uh oh!

millenomi commented Apr 3, 2018

Uh oh!

alblue commented Apr 12, 2018

Uh oh!

pvieito commented May 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

millenomi commented May 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

millenomi commented May 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felix91gr commented Jun 6, 2018

Uh oh!

millenomi commented Jun 8, 2018

Uh oh!

millenomi commented Jun 9, 2018

Uh oh!

millenomi commented Jun 11, 2018

Uh oh!

spevans commented Jun 12, 2018

Uh oh!

rudkx commented Jun 12, 2018

Uh oh!

millenomi commented Feb 23, 2018 •

edited

Loading

pvieito commented May 27, 2018 •

edited

Loading