Skip to content

Process: .currentDirectoryURL and .executableURL fixes #2525

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

Merged
merged 1 commit into from
Oct 22, 2019

Conversation

spevans
Copy link
Contributor

@spevans spevans commented Sep 28, 2019

  • currentDirectoryURL should be of type URL?

  • Disallow setting either property to nil or a non-File URL,
    to match Darwin.

  • Dont set PWD environment variable, it should just be copied from the
    current environment, to match Darwin.

@spevans
Copy link
Contributor Author

spevans commented Sep 28, 2019

@swift-ci test

@spevans spevans force-pushed the pr_process_current_directory branch from 442a0ab to 49f825e Compare September 29, 2019 22:19
@spevans
Copy link
Contributor Author

spevans commented Sep 29, 2019

@swift-ci test

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Sep 30, 2019

@swift-ci test

- currentDirectoryURL should be of type URL?

- Disallow setting either property to nil or a non-File URL,
  to match Darwin.

- Dont set PWD environment variable, it should just be copied from the
  current environment, to match Darwin.
@spevans spevans force-pushed the pr_process_current_directory branch from 49f825e to 2427fb2 Compare September 30, 2019 10:12
@spevans
Copy link
Contributor Author

spevans commented Sep 30, 2019

@swift-ci test

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Sep 30, 2019

@swift-ci test

get { _executable }
set {
guard let url = newValue, url.isFileURL else {
fatalError("must provide a launch path")
Copy link
Member

Choose a reason for hiding this comment

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

Is fatalError correct? Should this not be an exception of sorts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Darwin throws NSException so fatalError is the swift equivalent. Also it is a property so nothing else that could be done anyway.

get { _currentDirectoryURL }
set {
guard let url = newValue, url.isFileURL else {
fatalError("non-file URL argument")
Copy link
Member

Choose a reason for hiding this comment

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

Similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

@spevans
Copy link
Contributor Author

spevans commented Oct 22, 2019

@swift-ci test and merge

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Oct 22, 2019

@swift-ci test and merge

@swift-ci swift-ci merged commit e872f47 into swiftlang:master Oct 22, 2019
@compnerd
Copy link
Member

I think that this breaks the Windows build:

2019-10-23T06:22:34.1820412Z D:/a/1/s/swift-corelibs-foundation/Foundation/Process.swift:587:15: error: value of optional type 'URL?' must be unwrapped to refer to member 'path' of wrapped base type 'URL'
2019-10-23T06:22:34.1820755Z           try currentDirectoryURL.path.withCString(encodedAs: UTF16.self) { wszCurrentDirectory in
2019-10-23T06:22:34.1821080Z               ^

https://dev.azure.com/compnerd/3133d6ab-80a8-4996-ac4f-03df25cd3224/_apis/build/builds/12302/logs/95

@spevans
Copy link
Contributor Author

spevans commented Oct 24, 2019

@compnerd That's annoying, I will sort out a fix. I just realised I need to add an extra test for when .currentDirectoryURL is nil anyway so I can do that at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants