Skip to content

Incorporate feedback from 2nd review of SE-0292 #1319

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

Merged
merged 10 commits into from
May 27, 2021
Merged

Incorporate feedback from 2nd review of SE-0292 #1319

Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1947dce
Fix regular expression for package scopes
mattt Apr 5, 2021
5b568fa
Package names are compared using NFKC + CaseFolding
mattt Apr 5, 2021
4f019d3
The default, unscoped registry is denoted by [default]
mattt Apr 5, 2021
526fe33
Mention .netrc file as mitigation for hardcoded credentials
mattt Apr 5, 2021
79d1377
Remove trailing whitespace
mattt Apr 5, 2021
372c605
Fix typo name -> scope
mattt Apr 5, 2021
ac89166
Add link to draft registry proposal in @apple/swift-package-manager
mattt May 4, 2021
d44d47a
Update Information disclosure section
mattt May 18, 2021
ed0c440
Add section discussing authentication
mattt May 20, 2021
c03e612
Use console syntax for terminal code blocks
mattt May 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions proposals/0292-package-registry-service.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ and downloading the source archive for a release:
| `GET` | `/{scope}/{name}/{version}.zip` | Download source archive for a package release |
| `GET` | `/identifiers{?url}` | Lookup package identifiers registered for a URL |

A formal specification for the package registry interface
is provided alongside this proposal.
A formal specification for the package registry interface is provided
[alongside this proposal](https://github.com/apple/swift-package-manager/blob/main/Documentation/RegistryDraft.md).
In addition,
an OpenAPI (v3) document
and a reference implementation written in Swift
Expand All @@ -145,11 +145,11 @@ A package scope consists of
alphanumeric characters and hyphens.
Hyphens may not occur at the beginning or end,
nor consecutively within a scope.
The maximum length of a package name is 39 characters.
The maximum length of a package scope is 39 characters.
A valid package scope matches the following regular expression pattern:

```regexp
\A[a-zA-Z\d](?:[a-zA-Z\d]|-(?=[a-zA-Z\d])){0,39}\z
\A[a-zA-Z\d](?:[a-zA-Z\d]|-(?=[a-zA-Z\d])){0,38}\z
```

A package's *name* uniquely identifies a package in a scope.
Expand All @@ -163,16 +163,9 @@ A valid package name matches the following regular expression pattern:
> For more information,
> see [Unicode Identifier and Pattern Syntax][UAX31].

Package scopes are case-insensitive
(for example, `mona` ≍ `MONA`).
Package names are
case-insensitive,
diacritic-insensitive
(for example, `Å` ≍ `A`), and
width-insensitive
(for example, `Ａ` ≍ `A`).
Package names are compared using
[Normalization Form Compatible Composition (NFKC)][UAX15].
[Normalization Form Compatible Composition (NFKC)][UAX15]
with locale-independent case folding.

#### New `PackageDescription` API

Expand Down Expand Up @@ -242,7 +235,7 @@ that is, the `package` parameter in `.product(name:package)` method calls.

```diff
targets: [
.target(name: "MyLibrary",
.target(name: "MyLibrary",
dependencies: [
.product(name: "LinkedList",
- package: "LinkedList")
Expand Down Expand Up @@ -457,6 +450,8 @@ in the root directory of a package
creates or updates the `.swiftpm/config/registries.json` file
with a new top-level `registries` key
that's associated with an object containing the specified registry URLs.
The default, unscoped registry is associated with the key `[default]`.
Any scoped registries are keyed by their case-folded name.

For example,
a build server that doesn't allow external network connections
Expand All @@ -471,8 +466,8 @@ $ cat .swiftpm/config/registries.json
```json
{
"registries": {
"default": {
"url": "https://internal.example.com"
"[default]": {
"url": "https://internal.example.com"
}
},
"version": 1
Expand Down Expand Up @@ -507,7 +502,7 @@ $ cat .swiftpm/config/registries.json

```json
{
"registries": {
"registries": {
"example": {
"url": "https://internal.example.com"
}
Expand Down Expand Up @@ -557,8 +552,8 @@ consider the following global and local registry configuration files:
```jsonc
// Global configuration (~/.swiftpm/config/registries.json)
{
"registries": {
"default": {
"registries": {
"[default]": {
Copy link
Contributor

@tomerd tomerd May 3, 2021

Choose a reason for hiding this comment

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

does square brackets have precedence in other such system? in Swift we usually use underbar for making "special" things

Copy link
Contributor Author

@mattt mattt May 3, 2021

Choose a reason for hiding this comment

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

I chose [default], because it was visually distinctive. However, the only requirement is for it to not be a valid scope name, so _default could work just as well. At that point, it's a subjective choice.

As far as precedence, Swift uses square brackets in closures to annotate capture semantics ([weak self]), whereas underscore-prefixed variables are an unenforced convention. Another option to consider might be _. I don't feel too strongly either which way.

Copy link
Contributor

@tomerd tomerd May 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer we go with underbar _ or dot ., tho the latter may be too subtle. @neonichu @abertelrud @yim-lee @weissi @Lukasa opinions?

Copy link
Contributor

@yim-lee yim-lee May 4, 2021

Choose a reason for hiding this comment

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

+1 on _

Copy link
Contributor

@neonichu neonichu May 4, 2021

Choose a reason for hiding this comment

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

I think _ is usually associated with private or "not to be used" things in the Apple developer community, so seems like we should avoid that.

Copy link

@weissi weissi May 4, 2021

Choose a reason for hiding this comment

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

Agree with @neonichu , _ means "don't use, the ACL on this decl is more permissive than it should ideally be" in most of Swift.

Happy with [default], <default>, (default), .default, ... Don't think I have a strong preference. We could also go with "" to mean default but that's probably a bit opaque.

Copy link
Contributor

@Lukasa Lukasa May 26, 2021

Choose a reason for hiding this comment

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

Happy with any of @weissi's options there, I don't object to the _ but I understand the desire not to use it.

"url": "https://global.example.com"
},
"foo": {
Expand All @@ -570,7 +565,7 @@ consider the following global and local registry configuration files:

// Local configuration (.swiftpm/config/registries.json)
{
"registries": {
"registries": {
"foo": {
"url": "https://local.example.com"
}
Expand Down Expand Up @@ -786,7 +781,7 @@ and attempt to reuse those credentials to impersonate the user.
```json
{
"registries": {
"default": {
"[default]": {
"url": "https://<USERNAME>:<TOKEN>@swift.pkg.github.com/<OWNER>/"
}
},
Expand All @@ -796,7 +791,8 @@ and attempt to reuse those credentials to impersonate the user.
```

This kind of attack can be mitigated on an individual basis
by adding `.swiftpm/config` to a project's `.gitignore` file.
by adding `.swiftpm/config` to a project's `.gitignore` file
or storing credentials in a `.netrc` file.
The risk could be mitigated for all users
if Swift Package Manager included a `.gitignore` file
in its new project template.
Expand Down