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.

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

Ensure all packages are included in registry index, even if they have no versions #318

Merged
merged 11 commits into from
Feb 5, 2022

Conversation

thomashoneyman
Copy link
Member

Fixes #302 by ensuring that all packages with a valid package name are included in the registry index during the legacy import process. This does not ensure that these packages are uploaded (we're going to have to decide how we want to do that -- write an empty directory?), and the unused binding will leave a warning active until we have added them to the upload process.

@thomashoneyman thomashoneyman requested review from f-f and colinwahl and removed request for f-f February 2, 2022 00:13
@f-f
Copy link
Member

f-f commented Feb 2, 2022

Note that the Registry Index is not our source of truth, we just accidentally go through it while legacy-importing - I think that in order to consider #302 fixed we should write empty Metadata files for these packages.
This should happen in the LegacyImport side, since it's a thing only when we batch-import old names.

This is because "is a Metadata file there?" is our main check for registering a new name, see:
https://github.com/purescript/registry/blob/335c07cb00f3dada632019996d9f66eed9c6079e/ci/src/Registry/API.purs#L105-L123

@thomashoneyman
Copy link
Member Author

Ah, I see, I was aiming to write empty manifests — I can write the metadata files instead.

@thomashoneyman
Copy link
Member Author

thomashoneyman commented Feb 2, 2022

@f-f, @colinwahl I've updated this to write reserved package names to the metadata directory. In 33b23b3 I started tracking that directory, but I can revert the change if y'all don't feel ready to track this information or want to wait on #322 before tracking this directory.

The change is actually quite small -- just 3 files affected -- so I'd recommend reviewing by filtering only to .purs files.

-- We don't need to preserve any packages that made it through
-- processing into the resulting registry index.
guard $ isNothing $ Map.lookup parsedName registryIndex
{ owner, repo } <- hush $ GitHub.parseRepo address
Copy link
Member Author

@thomashoneyman thomashoneyman Feb 2, 2022

Choose a reason for hiding this comment

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

I opted to only reserve names for packages that have a valid GitHub repository as well as a valid package name, because that way we can write an empty metadata file for them.

This is acceptable because only one package has a malformed package name (TypeAhead) and it makes sense not to reserve that name (it can't be used anyway), and because zero packages have malformed GitHub URLs so no packages will fail this check.

@f-f
Copy link
Member

f-f commented Feb 3, 2022

I started tracking that directory, but I can revert the change if y'all don't feel ready to track this information or want to wait on purescript/registry#322 before tracking this directory.

#322 is about the registry-index, which is downstream for here. We are definitely good to start tracking the metadata directory here!

@thomashoneyman thomashoneyman requested a review from f-f February 3, 2022 17:14
Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

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

This is great, thanks!

@thomashoneyman thomashoneyman merged commit 62b93f1 into master Feb 5, 2022
@thomashoneyman thomashoneyman deleted the trh/ensure-all-packages-in-registry-index branch February 5, 2022 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify that we retain package names even if all versions are dropped
2 participants