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.

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

Jump to bottom

Multi-dot globbing #5372

Merged
merged 2 commits into from
Jun 16, 2018
Merged

Multi-dot globbing #5372

merged 2 commits into from
Jun 16, 2018

Conversation

quasicomputational
Copy link
Contributor

Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

New test added for the cabal check and specific tests for this behaviour have been added to the pre-existing glob tests.

The output of cabal check has changed a bit, but not for the worse, I don't think; it's now more like the other commands.

@quasicomputational quasicomputational mentioned this pull request Jun 10, 2018
Closed
4 tasks
23Skidoo
23Skidoo approved these changes Jun 11, 2018
View reviewed changes
Copy link
Member

@23Skidoo 23Skidoo left a comment

Choose a reason for hiding this comment

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

LGTM modulo minor comments.

Cabal/ChangeLog.md Outdated
@@ -34,6 +34,12 @@
* `install-includes` now works as expected with foreign libraries
([#5302](https://github.com/haskell/cabal/issues/5299)).
* Removed support for JHC.
* With `cabal-version: 3.0`, globs can now match only a suffix of
Copy link
Member

Choose a reason for hiding this comment

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

globs can now match only a suffix of extensions.

I'd reword this slightly: "when matching a wildcard plus extension, we no longer require the full extension to match exactly. For example, the *.gz glob pattern will now match foo.tar.gz and *.html will match foo.en.html."

Cabal/Distribution/Simple/Glob.hs
let (candidateBase, candidateExts) = splitExtensions (last $ seg:segs)
in ext == candidateExts && not (null candidateBase)
FinalMatch NonRecursive ext ->
guard (not (null candidateBase))
Copy link
Member

Choose a reason for hiding this comment

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

So *.html won't match .foo.html. Makes sense, since that's how it works in shell, but we should document it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already taken care of in d6b829d. As noted in that commit, this is the 2.2 behaviour.

@quasicomputational
Copy link
Contributor Author

Thanks! Rebased and squashed for tidiness and should be ready to merge, but I'll let it sit for a few days to see if anyone else spots anything that needs changing before the big green button gets pressed.

@quasicomputational @23Skidoo
Make cabal check play nice with cabal-testsuite.
185ad50 
Now `cabal check` will produce marked output and won't wrap lines if
you don't tell it to.
@quasicomputational @23Skidoo
Allow globs to match against a suffix of a file's extensions
bc3c477 
This has the effect of allowing a glob `*.html` to match the file
`foo.en.html`. For compatibility, this is only allowed with
`cabal-version: 3.0` or later; for earlier spec versions, a warning
will be generated by `cabal check` if there are files affected by this
change in behaviour.

Fixes haskell#5057. Fixes haskell#784. Closes haskell#5061.
@quasicomputational quasicomputational merged commit e4294f3 into haskell:master Jun 16, 2018
quasicomputational added a commit to quasicomputational/cabal that referenced this pull request Jun 26, 2018
@quasicomputational
Make Setup.hs configure more CWD-independent.
cb606d5 
Previously, we were checking the package with a hard-coded root
directory of ".". This was not a problem before, but with haskell#5372 we
have started to expand globs while checking packages, which breaks if
the CWD is not the directory containing the `.cabal` file and causes
snowleopard/hadrian#634.

Luckily, this is an easy fix: the correct directory is easy to
determine. Writing a test and making sure it's tickling the failing
case took longer than writing the fix!

"." is hard-coded as the root directory passed to `checkPackageFiles`
in a few other places, but those are (a) non-trivial to test, and (b)
already in places that have other assumptions about their CWD, so I
have simply documented the CWD requirement for those.
quasicomputational added a commit to quasicomputational/cabal that referenced this pull request Jun 26, 2018
@quasicomputational
Make Setup.hs configure more CWD-independent.
bf5a718 
Previously, we were checking the package with a hard-coded root
directory of ".". This was not a problem before, but with haskell#5372 we
have started to expand globs while checking packages, which breaks if
the CWD is not the directory containing the `.cabal` file and causes
snowleopard/hadrian#634.

Luckily, this is an easy fix: the correct directory is easy to
determine. Writing a test and making sure it's tickling the failing
case took longer than writing the fix!

"." is hard-coded as the root directory passed to `checkPackageFiles`
in a few other places, but those are (a) non-trivial to test, and (b)
already in places that have other assumptions about their CWD, so I
have simply documented the CWD requirement for those.
@quasicomputational quasicomputational mentioned this pull request Jun 26, 2018
Merged
4 tasks
quasicomputational added a commit to quasicomputational/cabal that referenced this pull request Jun 27, 2018
@quasicomputational
Make Setup.hs configure more CWD-independent.
7f0e066 
Previously, we were checking the package with a hard-coded root
directory of ".". This was not a problem before, but with haskell#5372 we
have started to expand globs while checking packages, which breaks if
the CWD is not the directory containing the `.cabal` file and causes
snowleopard/hadrian#634.

Luckily, this is an easy fix: the correct directory is easy to
determine. Writing a test and making sure it's tickling the failing
case took longer than writing the fix!

"." is hard-coded as the root directory passed to `checkPackageFiles`
in a few other places, but those are (a) non-trivial to test, and (b)
already in places that have other assumptions about their CWD, so I
have simply documented the CWD requirement for those.
quasicomputational added a commit to quasicomputational/cabal that referenced this pull request Jun 28, 2018
@quasicomputational
Ensure cabal check doesn't error out early on invalid globs
061073b 
This has been a problem since haskell#5372 began expanding globs in `cabal
check`. Now the logic of running a glob is separated from the parsing,
giving the caller the opportunity to handle parsing failures flexibly.
quasicomputational added a commit to quasicomputational/cabal that referenced this pull request Jul 4, 2018
@quasicomputational
Ensure cabal check doesn't error out early on invalid globs
eb4dcea 
This has been a problem since haskell#5372 began expanding globs in `cabal
check`. Now the logic of running a glob is separated from the parsing,
giving the caller the opportunity to handle parsing failures flexibly.
quasicomputational added a commit that referenced this pull request Jul 8, 2018
@quasicomputational
Make Setup.hs configure more CWD-independent.
a31ab06 
Previously, we were checking the package with a hard-coded root
directory of ".". This was not a problem before, but with #5372 we
have started to expand globs while checking packages, which breaks if
the CWD is not the directory containing the `.cabal` file and causes
snowleopard/hadrian#634.

Luckily, this is an easy fix: the correct directory is easy to
determine. Writing a test and making sure it's tickling the failing
case took longer than writing the fix!

"." is hard-coded as the root directory passed to `checkPackageFiles`
in a few other places, but those are (a) non-trivial to test, and (b)
already in places that have other assumptions about their CWD, so I
have simply documented the CWD requirement for those.
quasicomputational added a commit to quasicomputational/cabal that referenced this pull request Jul 8, 2018
@quasicomputational
Ensure cabal check doesn't error out early on invalid globs
24c4e77 
This has been a problem since haskell#5372 began expanding globs in `cabal
check`. Now the logic of running a glob is separated from the parsing,
giving the caller the opportunity to handle parsing failures flexibly.
typedrat pushed a commit that referenced this pull request Jul 9, 2018
@quasicomputational @typedrat
Ensure cabal check doesn't error out early on invalid globs
dcc157c 
This has been a problem since #5372 began expanding globs in `cabal
check`. Now the logic of running a glob is separated from the parsing,
giving the caller the opportunity to handle parsing failures flexibly.
@andreasabel andreasabel added the re: globbing Concerning issues with globbing file patterns label Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@23Skidoo 23Skidoo 23Skidoo approved these changes

Assignees
No one assigned
Labels
re: globbing Concerning issues with globbing file patterns
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quasicomputational @23Skidoo @andreasabel