Skip to content

Implement extra-files #603

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 1 commit into from
Mar 14, 2025
Merged

Implement extra-files #603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## Unreleased
- Add support for `extra-files` (see #603)

## Changes in 0.38.0
- Generate `build-tool-depends` instead of `build-tools` starting with
Expand Down
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ library:
| `build-type` | · | `Simple`, or `Custom` if `custom-setup` exists | Must be `Simple`, `Configure`, `Make`, or `Custom` | | |
| `extra-source-files` | · | | Accepts [glob patterns](#file-globbing) | | |
| `extra-doc-files` | · | | Accepts [glob patterns](#file-globbing) | | `0.21.2` |
| `extra-files` | · | | Accepts [glob patterns](#file-globbing) | | UNRELEASED |
| `data-files` | · | | Accepts [glob patterns](#file-globbing) | | |
| `data-dir` | · | | | | |
| `github` | `source-repository head` | | Accepts `owner/repo` or `owner/repo/subdir` | `github: foo/bar` |
Expand Down
6 changes: 6 additions & 0 deletions src/Hpack/Config.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ package name version = Package {
, packageFlags = []
, packageExtraSourceFiles = []
, packageExtraDocFiles = []
, packageExtraFiles = []
, packageDataFiles = []
, packageDataDir = Nothing
, packageSourceRepository = Nothing
Expand Down Expand Up @@ -602,6 +603,7 @@ data PackageConfig_ library executable = PackageConfig {
, packageConfigFlags :: Maybe (Map String FlagSection)
, packageConfigExtraSourceFiles :: Maybe (List FilePath)
, packageConfigExtraDocFiles :: Maybe (List FilePath)
, packageConfigExtraFiles :: Maybe (List FilePath)
, packageConfigDataFiles :: Maybe (List FilePath)
, packageConfigDataDir :: Maybe FilePath
, packageConfigGithub :: Maybe GitHub
Expand Down Expand Up @@ -831,6 +833,7 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
makeVersion [2,2] <$ guard mustSPDX
, makeVersion [1,24] <$ packageCustomSetup
, makeVersion [1,18] <$ guard (not (null packageExtraDocFiles))
, makeVersion [3,14] <$ guard (not (null packageExtraFiles))
, packageLibrary >>= libraryCabalVersion
, internalLibsCabalVersion packageInternalLibraries
, executablesCabalVersion packageExecutables
Expand Down Expand Up @@ -1024,6 +1027,7 @@ data Package = Package {
, packageFlags :: [Flag]
, packageExtraSourceFiles :: [Path]
, packageExtraDocFiles :: [Path]
, packageExtraFiles :: [Path]
, packageDataFiles :: [Path]
, packageDataDir :: Maybe FilePath
, packageSourceRepository :: Maybe SourceRepository
Expand Down Expand Up @@ -1272,6 +1276,7 @@ toPackage_ dir (Product g PackageConfig{..}) = do

extraSourceFiles <- expandGlobs "extra-source-files" dir (fromMaybeList packageConfigExtraSourceFiles)
extraDocFiles <- expandGlobs "extra-doc-files" dir (fromMaybeList packageConfigExtraDocFiles)
extraFiles <- expandGlobs "extra-files" dir (fromMaybeList packageConfigExtraFiles)

let dataBaseDir = maybe dir (dir </>) packageConfigDataDir

Expand Down Expand Up @@ -1316,6 +1321,7 @@ toPackage_ dir (Product g PackageConfig{..}) = do
, packageFlags = flags
, packageExtraSourceFiles = extraSourceFiles
, packageExtraDocFiles = extraDocFiles
, packageExtraFiles = extraFiles
, packageDataFiles = dataFiles
, packageDataDir = packageConfigDataDir
, packageSourceRepository = sourceRepository
Expand Down
1 change: 1 addition & 0 deletions src/Hpack/Render.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ renderPackageWith settings headerFieldsAlignment existingFieldOrder sectionsFiel
Field "tested-with" $ CommaSeparatedList packageTestedWith
, Field "extra-source-files" (renderPaths packageExtraSourceFiles)
, Field "extra-doc-files" (renderPaths packageExtraDocFiles)
, Field "extra-files" (renderPaths packageExtraFiles)
, Field "data-files" (renderPaths packageDataFiles)
] ++ maybe [] (return . Field "data-dir" . Literal) packageDataDir

Expand Down
14 changes: 14 additions & 0 deletions test/EndToEndSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,20 @@ spec = around_ (inTempDirectoryNamed "my-package") $ do
- "*.markdown"
|] `shouldWarn` ["Specified pattern \"*.markdown\" for extra-doc-files does not match any files"]

describe "extra-files" $ do
it "accepts a list of files" $ do
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
it "accepts a list of files" $ do
it "accepts extra-files" $ do

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We currently have:

extra-doc-files
  accepts a list of files

and

extra-files
  accepts extra-files

seemed a little 'circular' as an output - but if you are sure that is what you want, I'll put it through.

Copy link
Owner

Choose a reason for hiding this comment

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

The priorities are:

  1. It needs to be clear what the purpose of a specific test case is ("What are we testing here?").
  2. Consistency
  3. Anything else

As (1) is the main priority, I would shy away from sacrificing it for stylistic reasons.

In this specific case, we are testing that it "accepts extra-files". We are not concerned with whether it can be a list, whether it accept globs, etc. (those aspects are covered by other test cases).

seemed a little 'circular' as an output

Yes, definitely agree!

However, if you would want to address this, then I think the right approach would be to get rid of the describe "extra-files".

But then again we already have things like:

describe "spec-version" $ do
  it "accepts spec-version" $ do

So keeping the describe is at least consistent with what we already have (in line with our second priority).

Copy link
Owner

Choose a reason for hiding this comment

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

@mpilgrem did you read this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, my mistake. I'll fix it now.

touch "CHANGES.markdown"
touch "README.markdown"
[i|
extra-files:
- CHANGES.markdown
- README.markdown
|] `shouldRenderTo` (package [i|
extra-files:
CHANGES.markdown
README.markdown
|]) {packageCabalVersion = "3.14"}

describe "build-tools" $ do
context "with known build tools" $ do
context "when cabal-version < 2" $ do
Expand Down