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

Added dhall package command #2487

Merged
merged 6 commits into from
Feb 22, 2023
Merged

Conversation

mmhat
Copy link
Collaborator

@mmhat mmhat commented Feb 1, 2023

Fixes #1645

@mmhat mmhat requested a review from Gabriella439 February 1, 2023 19:14
@mmhat mmhat force-pushed the 1645-dhall-package branch from 40fd9d3 to 6a83ebb Compare February 1, 2023 21:01
@Gabriella439
Copy link
Collaborator

Sorry for the delay, but I think the command-line interface I would suggest is:

$ dhall package "${DIRECTORY}"  # Create a `package.dhall` in that directory from its contents
$ dhall packages "${FILES[@]}"  # Create a `package.dhall` from those files if they all share the same directory, otherwise fail

… possibly with an optional --name option to select a name other than package.dhall. But generally I feel like we should try to promote the convention that the package file is named package.dhall and make that as ergonomic as possible.

Another advantage of doing it this way is that you don't need to worry about computing relative paths from the input files to the output package file, so it simplifies the implementation a lot.

@mmhat
Copy link
Collaborator Author

mmhat commented Feb 11, 2023

@Gabriella439 Thanks for the feedback; Yes, that would indeed simplify the implementation. I am a bit hesitant to add more than one new top-level command to the CLI - especially since the two you proposed are quite similar. Why do we want to distinguish the directory/files cases anyway?

However, if we were deciding to do that I'd propose:

$ dhall package directory "${DIRECTORY}"
$ dhall package files "${FILES[@]}"

What do you think about that?

We require that all files are in the same directory.
The package.dhall will be written to that directory.
@mmhat
Copy link
Collaborator Author

mmhat commented Feb 11, 2023

I changed the implementation to allow only files from the same directory and it is closer to the suggested behavior now. I did not yet change the CLI as the discussion about that is ongoing.

@mmhat
Copy link
Collaborator Author

mmhat commented Feb 11, 2023

No idea why the tests are failing on the Hydra build. They succeed on all other runners and I can't reproduce the failure locally.

@Gabriella439
Copy link
Collaborator

this usually means that you need to add something to extra-source-files:

Extra-Source-Files:

… because the thing that the Nix build does differently is that it builds dhall from the source distribution that cabal would have generated and cabal will omit files that aren't explicitly mentioned in extra-source-files from the source distribution

@mmhat mmhat merged commit 4ce3e65 into dhall-lang:master Feb 22, 2023
@mmhat mmhat deleted the 1645-dhall-package branch February 22, 2023 12:47
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.

Automated packaging (dhall package)
2 participants