-
Notifications
You must be signed in to change notification settings - Fork 80
Require all packages to solve / compile and include all valid compilers in their metadata #669
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
Open
thomashoneyman
wants to merge
70
commits into
master
Choose a base branch
from
trh/compilers-in-metadata
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
70 commits
Select commit
Hold shift + click to select a range
0b6dd4e
Add 'compilers' field to metadata
thomashoneyman e15e4a8
Add utilities for building with many compilers
thomashoneyman d8e7e41
Remove PackageSource and require all packages to solve/compile
thomashoneyman 8e069b6
Determine all compilers for package in publish pipeline
thomashoneyman 5348ee2
Initial cut at discovering compiler in legacy import
thomashoneyman 630c0bf
Always look up metadata / manifests in each publishing step
thomashoneyman 77d6e68
Testing the pipeline...
thomashoneyman 8749bea
Better reporting of failures
thomashoneyman be93d18
Update union of package set / spago / bower deps, consider ranges in …
thomashoneyman 5a15433
Include spago.yaml files in legacy import
thomashoneyman 559275c
Retain compilation in cache
thomashoneyman 09d515a
Consider compilers when solving
thomashoneyman 98ef892
Rely on solver per-compiler instead of looking at metadata for compat…
thomashoneyman ae621da
Adjust unused dependency pruning to replace used transitive deps
thomashoneyman 5c54103
Remove unused functions
thomashoneyman 441b960
wip
thomashoneyman 3495edb
Use cache when finding first suitable compiler
thomashoneyman 7ceab4c
WIP: Include missing direct imports
thomashoneyman 3b85cd5
No longer try to insert missing dependencies
thomashoneyman 3fa90b5
Address internal comments
thomashoneyman 628fdf0
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman 0d3cef9
Re-enable comment
thomashoneyman 4e8cb87
Remove unnecessary
thomashoneyman d7c4180
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman 81c85a4
Fix 'removed packages' stats
thomashoneyman 10bccee
Feedback
thomashoneyman 26c5aa0
Always print publish stats
thomashoneyman b11917e
tweaks
thomashoneyman 3ddde82
Better publish stats formatting and write removals
thomashoneyman ec388d1
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman 5b17cb3
Update flake
thomashoneyman f924b31
Integrate inserting missing dependencies
thomashoneyman 3cdb9b9
Tweaks for efficiency
thomashoneyman d0181e5
(hopefully) final run of the importer
thomashoneyman 6f9f0cd
Update spec to note transitive dependencies requirement.
thomashoneyman 2721c6a
attempt to discover publish compiler with both legacy and current ind…
thomashoneyman f8d0f80
Tweaks
thomashoneyman e2d6e87
Patch some legacy manifests
thomashoneyman b8a21a8
Range tweaks for bolson/deku/rito
thomashoneyman 3d7ab49
Update to fix darwin support for spago builds
thomashoneyman 6bc8d09
Clean up publish stats
thomashoneyman 9acbc94
Enforce an explicit 0.13 date cutoff / core org cutoff
thomashoneyman d2c3b9a
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman bea2013
Move location check above manifest parse
thomashoneyman c942722
Merge branch 'master'
thomashoneyman 637a757
format
thomashoneyman ab184f2
Fix octokit codec merge error
thomashoneyman 9cc56e7
Revert "Fix octokit codec merge error"
thomashoneyman c05fcb9
Set compiler explicitly to 0.15.5
thomashoneyman 637488d
Tweaks
thomashoneyman 662dd00
Set all purs test compilers to 0.15.4 range
thomashoneyman 8156aa2
Update retry logic to fix integration test
thomashoneyman ed7913c
Complete run of legacy importer
thomashoneyman ec8e3ff
Format
thomashoneyman d7d5e49
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman 7d74da3
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman a3f086b
Update SPEC.md
f-f de7c6e3
Add 'purescript' to the list of reserved packages
f-f 8c8d728
Move to NonEmpty in dhall types
f-f 7b57771
compilers is a NonEmptyArray
f-f e3d484b
Merge branch 'master' into trh/compilers-in-metadata
f-f 343b90b
Fix tests
f-f 67c7cb5
Fix tests
f-f 88dcffe
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman dec066b
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman e863d6b
fix e2e tests
thomashoneyman f27cd33
Merge branch 'master' into trh/compilers-in-metadata
thomashoneyman beb8d93
update flake to latest
thomashoneyman a55bef6
add an agents file
thomashoneyman 5ab364a
add archive seeder script
thomashoneyman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ result* | |
| *.sqlite3 | ||
| *.sqlite3-wal | ||
| *.sqlite3-shm | ||
|
|
||
| TODO.md | ||
| .spec-results | ||
|
|
||
| # Keep it secret, keep it safe. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| # AGENTS.md | ||
|
|
||
| The PureScript Registry implements a package registry for PureScript. See @SPEC.md for the registry specification and @CONTRIBUTING.md for detailed contributor documentation. | ||
|
|
||
| ## Development Environment | ||
|
|
||
| This project uses Nix with direnv. You should already be in the Nix shell automatically when entering the directory. If not, run: | ||
|
|
||
| ```sh | ||
| nix develop | ||
| ``` | ||
|
|
||
| ### Build and Test | ||
|
|
||
| The registry is implemented in PureScript. Use spago to build it and run PureScript tests. These are cheap and fast and should be used when working on the registry packages. | ||
|
|
||
| ```sh | ||
| spago build # Build all PureScript code | ||
| spago test # Run unit tests | ||
| ``` | ||
|
|
||
| Integration tests require two terminals (or the use of test-env in detached mode). The integration tests are only necessary to run if working on the server (app). | ||
|
|
||
| ```sh | ||
| # Terminal 1: Start test environment (wiremock mocks + registry server on port 9000) | ||
| nix run .#test-env | ||
|
|
||
| # Terminal 2: Run E2E tests once server is ready | ||
| spago run -p registry-app-e2e | ||
| ``` | ||
|
|
||
| Options: `nix run .#test-env -- --tui` for interactive TUI, `-- --detached` for background mode. | ||
|
|
||
| #### Smoke Test (Linux only) | ||
|
|
||
| The smoke test verifies that the server comes up properly and tests deployment. Only run this test if you are making changes which could break the deployment of the server. | ||
|
|
||
| ```sh | ||
| nix build .#checks.x86_64-linux.smoke -L | ||
| ``` | ||
|
|
||
| #### Continuous Integration via Nix Checks | ||
|
|
||
| There is a full suite of checks implemented with Nix which verify that packages build, formatting is correct, registry types are Dhall-conformant, and more. This is the primary check run in CI. | ||
|
|
||
| ```sh | ||
| nix flake check -L | ||
| ``` | ||
|
|
||
| ## Formatting | ||
|
|
||
| ```sh | ||
| # Format PureScript | ||
| purs-tidy format-in-place app app-e2e foreign lib scripts | ||
| purs-tidy check app app-e2e foreign lib scripts | ||
|
|
||
| # Format Nix files | ||
| nixfmt *.nix nix/**/*.nix | ||
| ``` | ||
|
|
||
| ## Project Structure | ||
|
|
||
| - `app/` — Registry server implementation. | ||
| - `app-e2e/` — E2E tests for the server API. | ||
| - `lib/` — **Public library** for consumers (Spago, Pursuit, etc.). Only types and functions useful to external tools belong here. Avoid implementation-specific code. | ||
| - `foreign/` — FFI bindings to JavaScript libraries. | ||
| - `scripts/` — Runnable modules for registry tasks (LegacyImporter, PackageTransferrer, PackageSetUpdater, etc.). Run via `nix run .#legacy-importer`, etc. | ||
| - `test-utils/` — Shared test utilities. | ||
| - `db/` — SQLite schemas and migrations (use `dbmate up` to initialize). | ||
| - `types/` — Dhall type specifications. | ||
| - `nix/` — Nix build and deployment configuration. | ||
|
|
||
| ## Scripts & Daily Workflows | ||
|
|
||
| The `scripts/` directory contains modules run as daily jobs by the purescript/registry repository: | ||
|
|
||
| - `LegacyImporter` — imports package versions from legacy Bower registry | ||
| - `PackageTransferrer` — handles package transfers | ||
| - `PackageSetUpdater` — automatic daily package set updates | ||
|
|
||
| Run scripts via Nix: `nix run .#<kebab-case-name>` (e.g., `nix run .#legacy-importer`). All scripts support `--help` for usage information. | ||
|
|
||
| ## Scratch Directory & Caching | ||
|
|
||
| The `scratch/` directory (gitignored) is used by scripts for: | ||
| - `.cache/` — Cached API responses, downloaded packages, etc. | ||
| - `logs/` — Log files | ||
| - `registry/`, `registry-index/` — Local clones for testing, also modified and optionally committed to by scripts | ||
|
|
||
| Caching is critical for the legacy importer due to the expense of downloading packages. The `Registry.App.Effect.Cache` module handles caching. | ||
|
|
||
| ## PureScript Conventions | ||
|
|
||
| ### Custom Prelude | ||
|
|
||
| Always use `Registry.App.Prelude` in `app/` and `app-e2e/` directories: | ||
|
|
||
| ```purescript | ||
| import Registry.App.Prelude | ||
| ``` | ||
|
|
||
| ### Effects via Run | ||
|
|
||
| Use the `run` library for extensible effects. Do NOT perform HTTP calls, console logs, or other effects directly in `Aff`. Check for existing effects in `app/src/App/Effect/` or consider adding one. | ||
|
|
||
| ### Import Style | ||
|
|
||
| Import types unqualified, values qualified. Use shortened module names: | ||
|
|
||
| ```purescript | ||
| import Registry.App.Prelude | ||
|
|
||
| import Data.Array as Array | ||
| import Data.String as String | ||
| import Node.FS.Aff as FS.Aff | ||
| import Parsing (Parser) | ||
| import Parsing as Parsing | ||
| import Parsing.Combinators as Parsing.Combinators | ||
| import Registry.Operation (AuthenticatedData) | ||
| import Registry.SSH as SSH | ||
| ``` | ||
|
|
||
| ## Deployment | ||
|
|
||
| Continuous deployment via GitHub Actions on master. Manual deploy: | ||
|
|
||
| ```sh | ||
| colmena apply | ||
| ``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| { | ||
| "name": "purescript-transitive", | ||
| "homepage": "https://github.com/purescript/purescript-transitive", | ||
| "license": "BSD-3-Clause", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/purescript/purescript-transitive.git" | ||
| }, | ||
| "dependencies": { | ||
| "purescript-effect": "^4.0.0" | ||
| } | ||
| } |
6 changes: 6 additions & 0 deletions
6
app/fixtures/github-packages/transitive-1.0.0/src/Transitive.purs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| module Transitive where | ||
|
|
||
| import Prelude | ||
|
|
||
| uno :: Int | ||
| uno = one |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be tidier to only allow a non-empty array instead of several possible types? After all, the state with multiple compilers listed is going to be a superset of the first state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue with the non-empty array is that it isn't clear whether an array of a single element represents one of:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When are we going to end up in a situation where we don't test the package against the whole set of compilers? My reading of the PR is that we always do?
In any case, we'll always have packages that are not "tested against the full set of compilers": when a new compiler version comes out, then all packages will need a retest, and if a package doesn't have the new compiler in the array then we don't know if it's not compatible or if it hasn't been tested yet.
Maybe we need another piece of state somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as implemented here we just go ahead and test everything as soon as we've published. However, I split out the state because in our initial discussions we worried about how long it takes for the compiler builds to run (it takes publishing from N seconds to N minutes in some cases — large libraries or ones that leverage a lot of type machinery). We'd originally talked about the compiler matrix being a cron job that runs later in the day. I just made it part of the publishing pipeline directly because it was simpler to implement.
If we decide that it's OK for publishing to take a long time then we can eliminate this state and just test the compilers immediately. In that case we'd just have a non-empty array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, that's a good point. You don't know if the metadata you're reading just hasn't been reached yet by an ongoing mass compiler build to check a new compiler.
Off the top of my head I don't know a good place to put some state about possible compiler support; the metadata files are not helpful if a new compiler comes out and we're redoing the build since they're only aware of the one package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm cool with this if you are.
We could either a) say that the supported list of compilers for a package can potentially be missing the current compiler if the matrix is currently running and not bother with state or b) put a JSON file or something in the metadata directory that indicates whether the compiler matrix is running. Then consumers can look at that.
Personally the matrix runs infrequently enough (just new compiler releases!) that I would rather opt for (a).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pondered this for a few days and I think it's complicated?
Since we're going towards a model where we'd only run one registry job at a time and queue the rest (to prevent concurrent pushes to the repo), I'm afraid that running the whole matrix at once would make publishing very slow.
Something that we could do to counteract this could be to split the "publish" and the "matrix runs": on publishing we'd just add the package metadata with one compiler, and at the end of the publishing job we'd queue a series of "compiler matrix" jobs, each testing one compiler. These jobs would be of low priority, so new publishes would get in front of the queue, and things can stay snappy.
The approach detailed above implies that we're in a world where we do (a), i.e. the list of compilers is always potentially out of date, and that's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional note about the above: since the above would be introducing an "asynchronous matrix builder", we need to consider the dependency tree in our rebuilding: if a package A is published with compiler X, and then a package B depending on it is immediately published after it (a very common usecase since folks seem to publish their packages in batches), then we'd need to either make sure that matrix-build jobs for B are always run after matrix-build jobs for A, or retry them somehow.