Skip to content

Make cabal check reject packages with invalid file names #10295

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

Closed
phadej opened this issue Aug 30, 2024 · 4 comments · Fixed by #10816
Closed

Make cabal check reject packages with invalid file names #10295

phadej opened this issue Aug 30, 2024 · 4 comments · Fixed by #10816

Comments

@phadej
Copy link
Collaborator

phadej commented Aug 30, 2024

Describe the feature request

Make cabal check reject packages when source distribution contains files with invalid names on any operating system. For example aux is not valid on Windows (and Windows file systems are case insensitive)., so e.g. module Foo.Aux.Helpers or Language.Con, Foreign.Nul are a bad idea.

Additional context

I think calling explicitly, System.FilePath.Posix.isValid and System.FilePath.Windows.isValid would be enough.

@ulysses4ever
Copy link
Collaborator

Sounds reasonable. @ffaf1, any thoughts?

@ffaf1
Copy link
Collaborator

ffaf1 commented Aug 31, 2024

It is a sensible request. We already do some isValid checks with regard to Windows

checkPackageId :: Monad m => PackageIdentifier -> CheckM m ()
checkPackageId (PackageIdentifier pkgName_ _pkgVersion_) = do
checkP
(not . FilePath.Windows.isValid . prettyShow $ pkgName_)
(PackageDistInexcusable $ InvalidNameWin pkgName_)
checkP (isPrefixOf "z-" . prettyShow $ pkgName_) $
(PackageDistInexcusable ZPrefix)

We just need to get this right with regard to — say — excluding autogenerated modules and the like.

@phadej
Copy link
Collaborator Author

phadej commented Aug 31, 2024

We just need to get this right with regard to — say — excluding autogenerated modules

You don't need to exclude them. Autogenerated modules will also have files on the filesystem. (EDIT: when autogenerated...)

mpickering added a commit that referenced this issue Mar 5, 2025
This change ensures that Cabal warns about modules with names that would cause
portability issues, especially on Windows systems where certain names like "aux",
"con", "nul", etc. are reserved by the operating system.

Fixes #10295
mpickering added a commit that referenced this issue Mar 5, 2025
This change ensures that Cabal warns about modules with names that would cause
portability issues, especially on Windows systems where certain names like "aux",
"con", "nul", etc. are reserved by the operating system.

Fixes #10295
mpickering added a commit that referenced this issue Mar 5, 2025
This change ensures that Cabal warns about modules with names that would cause
portability issues, especially on Windows systems where certain names like "aux",
"con", "nul", etc. are reserved by the operating system.

Fixes #10295
mpickering added a commit that referenced this issue Mar 5, 2025
This change ensures that Cabal warns about modules with names that would cause
portability issues, especially on Windows systems where certain names like "aux",
"con", "nul", etc. are reserved by the operating system.

Fixes #10295
@mpickering
Copy link
Collaborator

I have put up a MR #10816

Let me know if you think this is along the right lines. There are already quite comprehensive checks in place for most of the paths you can write in a cabal file but not the module names.

mpickering added a commit that referenced this issue Mar 5, 2025
This change ensures that Cabal warns about modules with names that would cause
portability issues, especially on Windows systems where certain names like "aux",
"con", "nul", etc. are reserved by the operating system.

Fixes #10295
mpickering added a commit that referenced this issue Mar 10, 2025
This change ensures that Cabal warns about modules with names that would cause
portability issues, especially on Windows systems where certain names like "aux",
"con", "nul", etc. are reserved by the operating system.

Fixes #10295
@mergify mergify bot closed this as completed in #10816 Apr 13, 2025
@mergify mergify bot closed this as completed in faa3c6a Apr 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants