Add support for Netrc for Downloader #2833

sstadelman · 2020-07-28T16:42:37Z

Implemented handling for netrc files. Matches connection settings for binary artifact host, and returns Basic authentication header to be appended to internal URLRequest of the Downloader. Will resolve Supporting basic auth for non-git binary dependency hosts.

Netrc parser based on Carthage implementation and tests, and migrated to regex for flexibility.

Features supported

Parse Netrc string text: single and multi-line
Match machine, login, password key values
Ignore comments
Ignore account and macdef entries (relevant only to ftp remotes)
Accept any order of login, password, account key values
Match default connection settings.
Validates single instance of default connection setting, and that default connection setting is last entry
Checks for netrc file in NSHomeDirectory()/.netrc by default
Workspace supports custom netrcFilePath parameter

Sequence Flow (including SPM)

In SPM, a netrc file absolute path is resolved during (3) getActiveWorkspace() call of run() procedure, from optional command argument (1) or NSHomeDirectory()/.netrc (4). Resolved absolute path is passed to initialized Workspace(5).

If binary artifacts should be updated, then the (7) download(...) operation attempts to load the netrc machine definitions from a file at the resolved netrcFilePath(8). The optional resulting Netrc instance is passed to the created FoundationDownloader for each artifact (11). The downloader attempts to match a credential in the list of the Netrc instance's Machine definitions for the artifact host (13). If matched, the base64-encoded <user>:<password> pair is set to the Authorization header for the respective URLRequest.

Notes

The main question wrt API addition was: how to efficiently pass the netrc content to the Downloader protocol, which is lightweight by design. Though netrc is a common technique, it is not necessarily a universal solution--perhaps in the future we should also support application tokens, etc., which might similarly pollute the protocol. So, this implementation introduces the AuthorizationProviding interface, under which additional authentication techniques could in the future be added to the Workspace, and injected to the binary update process in Downloader implementations.

public protocol AuthorizationProviding {
    /// Optional `Authorization` header, likely added to `URLRequest`
    func authorization(for url: Foundation.URL) -> String?
}

Downloader is extended as follows:

/// The `Downloader` protocol abstract away the download of a file with a progress report.
public protocol Downloader {
    /**/
    /// Downloads a file and keeps the caller updated on the progress and completion.
    ///
    /// - Parameters:
    ///   - url: The `URL` to the file to download.
    ///   - destination: The `AbsolutePath` to download the file to.
    ///   - authorizationProvider: Optional provider supplying `Authorization` header to be added to `URLRequest`.
    ///   - progress: A closure to receive the download's progress as number of bytes.
    ///   - completion: A closure to be notifed of the completion of the download.
    func downloadFile(
        at url: Foundation.URL,
        to destination: AbsolutePath,
        withAuthorizationProvider authorizationProvider: AuthorizationProviding?,
        progress: @escaping Progress,
        completion: @escaping Completion
    )
}

The netrc credential injection is added to FoundationDownloader. Other Downloaders should implement handling according to their requirements.
Requires OSX 10.13. Will obligate swift-driver and swift-tools-support-core packages to raise minimum to 10.13 also. Discussed in thread.

sstadelman · 2020-07-28T16:45:29Z

@neonichu depends on swiftlang/swift-driver#185.

sstadelman · 2020-07-29T17:07:08Z

@swift-ci please test

sstadelman · 2020-07-29T18:39:19Z

Please test with following PR:
swiftlang/swift-tools-support-core#88
swiftlang/swift-driver#185

@swift-ci Please test macOS platform

sstadelman · 2020-08-06T18:12:02Z

@neonichu is there anything I can do to help get this in the queue? I've got a product release in October which might be dependent on it.

sstadelman · 2020-08-11T17:09:13Z

Please test with following PR:
swiftlang/swift-tools-support-core#88
swiftlang/swift-driver#185

@swift-ci Please test

neonichu

LGTM except for one comment. Thank you very much for the contribution, @sstadelman

neonichu · 2020-08-18T17:35:22Z

Sources/Commands/SwiftTool.swift

+                    usage: "Absolute path to netrc file for downloading binary target artifacts"),
+                to: { $0.netrcFilePath = $1.path })
+        }
+        #endif


I think we should have the option always be present and emit an error if it is being used on non-macOS. It doesn't really matter right now since binaries aren't supported on non-macOS, but I think silently ignored options can be confusing.

Added check to post-processing block
https://github.com/apple/swift-package-manager/pull/2833/files#diff-b1d92ba41a0ed8ddabdf8c2604a41504R572-R584

neonichu · 2020-08-18T17:37:25Z

Please test with following PR:
swiftlang/swift-tools-support-core#88
swiftlang/swift-driver#185

@swift-ci Please smoke test

sstadelman · 2020-08-18T22:17:45Z

@neonichu made change requested, also synced changes from swift-tools-support-core #88

Since that is merged, and swift-driver PR is cancelled, can test/smoke-test directly.

MaxDesiatov · 2020-08-18T22:20:15Z

@swift-ci please smoke test

neonichu · 2020-08-19T05:49:21Z

Sorry for the conflicts, but I needed to unblock the build in #2876 separately from this.

sstadelman · 2020-08-19T15:29:26Z

@neonichu do you need me to clean up these conflicts?

MaxDesiatov · 2020-08-19T15:30:13Z

Yes, please clean up the conflicts, I'll be able to trigger CI then.

sstadelman · 2020-08-19T18:11:37Z

@MaxDesiatov resolved conflicts

MaxDesiatov · 2020-08-19T18:12:15Z

@swift-ci please smoke test

sstadelman · 2020-08-24T18:19:56Z

@MaxDesiatov I see it passing on self-hosted linux. I guess there's an integration issue elsewhere.

neonichu · 2020-08-24T18:22:29Z

Probably transient failures, let's try again.

neonichu · 2020-08-24T18:22:37Z

@swift-ci please smoke test

MaxDesiatov · 2020-08-25T19:58:00Z

@swift-ci please smoke test

MaxDesiatov · 2020-08-26T08:09:50Z

@swift-ci please smoke test

neonichu · 2020-08-26T23:07:58Z

Sorry @sstadelman, I'm afraid that merging the swift-argument-parser PR has made this conflict yet again :/

sstadelman · 2020-08-27T02:28:59Z

Np I’ll get it Thursday evening PT.

…

Sent from my iPhone

On Aug 26, 2020, at 4:08 PM, Boris Bügling ***@***.***> wrote: Sorry @sstadelman, I'm afraid that merging the swift-argument-parser PR has made this conflict yet again :/ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

sstadelman · 2020-08-28T05:33:16Z

@neonichu gtg

MaxDesiatov · 2020-08-28T07:34:40Z

@swift-ci please smoke test

MaxDesiatov · 2020-08-28T08:35:33Z

PackageToolTests.testNetrcFile is failing on Linux now.

sstadelman · 2020-08-31T20:11:18Z

I'm having trouble running in docker, or Xcode 11.6, due to missing @_spm tag. This isn't a problem in swift-tools-support-core. Is there a way to avoid these conflicts?

This is my current environment:
# TO LAUNCH DOCKER SWIFT ENV (from package root)
docker run --rm --privileged --interactive --tty --volume "$(pwd):/src" --workdir "/src" swift:latest

# FROM INSIDE DOCKER
apt-get update && apt-get upgrade
apt-get install libsqlite3-dev
swift test --enable-test-discovery --filter CommandTests.PackageToolTests

@neonichu sorry for a noob question, but I’m stuck getting my Linux environment up and running as configured above, since there seems to be a dependency in swift-driver on 5.3 features which are not available in Linux distribution. This has got me temporarily blocked. Is there any guidance you can share for the spm Linux environment setup, so I can get this thing free again?

MaxDesiatov · 2020-08-31T20:13:36Z

Did you try using 5.3 nightly images?

swift:latest points to the latest stable version, not a 5.3 snapshot, which seems to be required to build this PR.

…access https://forums.swift.org/t/spm-support-basic-auth-for-non-git-binary-dependency-hosts/37878

sstadelman · 2020-09-02T03:52:52Z

@MaxDesiatov good to go!

for posterity, here's the linux docker setup:

# TO LAUNCH DOCKER SWIFT ENV (from package root)
docker pull swiftlang/swift:nightly-5.3-bionic
docker run --rm --privileged --interactive --tty --volume "$(pwd):/src" --workdir "/src" swiftlang/swift:nightly-5.3-bionic

# FROM INSIDE DOCKER
apt-get update && apt-get upgrade
apt-get install libsqlite3-dev
apt-get install libncurses5-dev
swift test --enable-test-discovery --filter CommandsTests.PackageToolTests

MaxDesiatov · 2020-09-02T06:53:16Z

@swift-ci please smoke test

MaxDesiatov · 2020-09-02T09:12:04Z

@sstadelman thank you, all passes now.

@neonichu would you be able to have yet another look and merge if everything's good to go?

neonichu · 2020-09-03T21:45:47Z

Thanks @sstadelman, this is great.

sstadelman · 2020-09-03T21:50:19Z

Thanks again @neonichu and @MaxDesiatov, really appreciated your help and attention on this feature! 🍻

sstadelman requested review from abertelrud, aciidgh, friedbunny and neonichu as code owners July 28, 2020 16:42

sstadelman mentioned this pull request Jul 28, 2020

Add support for Netrc for Downloader swiftlang/swift-tools-support-core#88

Merged

sstadelman mentioned this pull request Jul 28, 2020

Bumped minimum platform version to 10.13 swiftlang/swift-driver#185

Merged

artemcm mentioned this pull request Jul 29, 2020

Bump minimum platform version to 10.13 #2837

Closed

sstadelman mentioned this pull request Jul 29, 2020

Revert #185: Bump minimum platform version to 10.13 swiftlang/swift-driver#187

Merged

sstadelman force-pushed the master branch from f541ebd to df7e46e Compare August 6, 2020 18:09

sstadelman force-pushed the master branch from df7e46e to c640d45 Compare August 11, 2020 17:05

sstadelman force-pushed the master branch from c640d45 to 18e4a0a Compare August 11, 2020 19:53

sstadelman changed the title ~~[Ready for Review] Add support for Netrc for Downloader~~ [Do not merge] Add support for Netrc for Downloader Aug 11, 2020

neonichu approved these changes Aug 18, 2020

View reviewed changes

sstadelman force-pushed the master branch from 18e4a0a to 6683021 Compare August 18, 2020 22:12

neonichu mentioned this pull request Aug 18, 2020

repair the build after apple/swift-tools-support-core#88 #2875

Closed

sstadelman force-pushed the master branch from fd8ea37 to b384c57 Compare August 19, 2020 18:05

sstadelman changed the title ~~[Do not merge] Add support for Netrc for Downloader~~ Add support for Netrc for Downloader Aug 19, 2020

sstadelman force-pushed the master branch 2 times, most recently from 1b98c58 to 136b515 Compare August 28, 2020 05:25

sstadelman added 5 commits September 1, 2020 15:47

working with custom netrc file location

147bc2f

Added support to TSC for using Netrc credentials for binary artifact …

9e39130

…access https://forums.swift.org/t/spm-support-basic-auth-for-non-git-binary-dependency-hosts/37878

eliminated dependency on NSHomeDirectory

320e6b9

cleaned up cruft from support core copy

7089e73

test case emits error on unsupported OS for --netrc-file option

bcfc6f8

sstadelman force-pushed the master branch from 1d8f1f4 to bcfc6f8 Compare September 2, 2020 03:42

neonichu merged commit 7387fc7 into swiftlang:master Sep 3, 2020

1ec5 mentioned this pull request Sep 16, 2020

Support installation via Swift Package Manager mapbox/mapbox-navigation-ios#2629

Closed

16 tasks

sstadelman added a commit to sstadelman/swift-package-manager that referenced this pull request Sep 24, 2020

Add support for Netrc for Downloader (cherry-pick swiftlang#2833)

5fa83bd

sstadelman mentioned this pull request Sep 24, 2020

Add support for Netrc for Downloader (cherry-pick #2833) #2955

Closed

azarovalex mentioned this pull request Dec 12, 2023

Use default authorizationProvider to support netrc and keychain credentials for private packages giginet/Scipio#111

Merged

Add support for Netrc for Downloader #2833

Add support for Netrc for Downloader #2833

Uh oh!

Conversation

sstadelman commented Jul 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Features supported

Sequence Flow (including SPM)

Notes

Uh oh!

sstadelman commented Jul 28, 2020

Uh oh!

sstadelman commented Jul 29, 2020

Uh oh!

sstadelman commented Jul 29, 2020

Uh oh!

sstadelman commented Aug 6, 2020

Uh oh!

sstadelman commented Aug 11, 2020

Uh oh!

neonichu left a comment

Choose a reason for hiding this comment

Uh oh!

neonichu Aug 18, 2020

Choose a reason for hiding this comment

Uh oh!

sstadelman Aug 18, 2020

Choose a reason for hiding this comment

Uh oh!

neonichu commented Aug 18, 2020

Uh oh!

sstadelman commented Aug 18, 2020

Uh oh!

MaxDesiatov commented Aug 18, 2020

Uh oh!

neonichu commented Aug 19, 2020

Uh oh!

sstadelman commented Aug 19, 2020

Uh oh!

MaxDesiatov commented Aug 19, 2020

Uh oh!

sstadelman commented Aug 19, 2020

Uh oh!

MaxDesiatov commented Aug 19, 2020

Uh oh!

sstadelman commented Aug 24, 2020

Uh oh!

neonichu commented Aug 24, 2020

Uh oh!

neonichu commented Aug 24, 2020

Uh oh!

MaxDesiatov commented Aug 25, 2020

Uh oh!

MaxDesiatov commented Aug 26, 2020

Uh oh!

neonichu commented Aug 26, 2020

Uh oh!

sstadelman commented Aug 27, 2020 via email

Uh oh!

sstadelman commented Aug 28, 2020

Uh oh!

MaxDesiatov commented Aug 28, 2020

Uh oh!

MaxDesiatov commented Aug 28, 2020

Uh oh!

sstadelman commented Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaxDesiatov commented Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sstadelman commented Sep 2, 2020

Uh oh!

MaxDesiatov commented Sep 2, 2020

Uh oh!

MaxDesiatov commented Sep 2, 2020

Uh oh!

neonichu commented Sep 3, 2020

Uh oh!

sstadelman commented Sep 3, 2020

sstadelman commented Jul 28, 2020 •

edited

Loading

sstadelman commented Aug 31, 2020 •

edited

Loading

MaxDesiatov commented Aug 31, 2020 •

edited

Loading