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

Handle case-insensitive file systems in a better way #5719

Open
andrejbauer opened this issue Jan 2, 2022 · 15 comments
Open

Handle case-insensitive file systems in a better way #5719

andrejbauer opened this issue Jan 2, 2022 · 15 comments
Labels
filesystem case-sensitivity Concerning case-(in)sensitivity of file system platform:macOS Any issue specific to macOS status: info-needed More information is needed from the bug reporter to confirm the issue.
Milestone

Comments

@andrejbauer
Copy link
Contributor

andrejbauer commented Jan 2, 2022

Where do I report bugs?

open import Relation.Binary.PropositionalEquality

module Error where

  data N : Set where
    Z : N
    S : N  N

  infixr 5 _+_

  _+_ : N  N  N
  m + Z = m
  m + S n = S (m + n)

  problem :  a  S Z + a ≡ S a
  problem Z = refl
  problem (S a) = trans (problem (S a)) refl

-- An internal error has occurred. Please report this as a bug.
-- Location of the error: __IMPOSSIBLE__, called at src/full/Agda/TypeChecking/Errors.hs:296:18 in
-- Agd-2.6.2-7cfdab78:Agda.TypeChecking.Errors

System info:

% uname -a
Darwin OA 20.6.0 Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021; root:xnu7195.141.6~3/RELEASE_X86_64 x86_64

% agda --version
Agda version 2.6.2

Do you need anything else?

@Trebor-Huang
Copy link

A random guess: since you are on a case-insensitive system, does your module name agree exactly with your file name (including case)? See #5508

@jespercockx
Copy link
Member

I failed to reproduce the error on Linux, so it could be something platform-related indeed. Can someone on Apple try if they can reproduce this?

@jespercockx jespercockx added status: info-needed More information is needed from the bug reporter to confirm the issue. platform:macOS Any issue specific to macOS labels Jan 3, 2022
@jespercockx jespercockx added this to the 2.6.3 milestone Jan 3, 2022
@andreasabel
Copy link
Member

A random guess: since you are on a case-insensitive system, does your module name agree exactly with your file name (including case)? See #5508

Yes, @andrejbauer :

  • What is the filename of module Error?
  • Does the issue go away with Agda-2.6.2.1?

@andreasabel andreasabel added the status: abandoned Work on this issue has been abandoned (not in changelog) label Jan 25, 2022
@andreasabel
Copy link
Member

The is likely no MWE coming. Closing.

@andrejbauer
Copy link
Contributor Author

Sorry for the delay. Using Agda 2.6.2 on MacOS, I can confirm that the problem is caused by case-insensitive file names. When I make sure that the file name matches, I get a reasonable error (about termination checking).

Is there a way for Agda to fail more graciously, or should we just blame Apple for this bug?

@gallais
Copy link
Member

gallais commented Jan 26, 2022

Looks like Agda.Interaction.FindFile already has the right logic. Should this ifdef:

#ifdef mingw32_HOST_OS
doesFileExistCaseSensitive f = do
doesFileExist f `and2M` do
bracket (findFirstFile f) (findClose . fst) $
fmap (takeFileName f ==) . getFindDataFileName . snd
#else
doesFileExistCaseSensitive = doesFileExist
#endif

also be activated for mac?

@gallais gallais reopened this Jan 26, 2022
@gallais gallais added filesystem case-sensitivity Concerning case-(in)sensitivity of file system and removed status: abandoned Work on this issue has been abandoned (not in changelog) labels Jan 26, 2022
@andreasabel
Copy link
Member

Sorry for the delay. Using Agda 2.6.2 on MacOS, I can confirm that the problem is caused by case-insensitive file names. When I make sure that the file name matches, I get a reasonable error (about termination checking).

Is there a way for Agda to fail more graciously, or should we just blame Apple for this bug?

No, sorry @andrejbauer, this it totally Agda's fault. I am on macOS myself, so I would be curious to reproduce the issue, but I was lacking information (i.e., the spelling of the filename of the module). Issue #5508 could be related, which is fixed on Agda 2.6.2.1. Could you please try to reproduce the issue on 2.6.2.1 (or complete the information so I can reproduce it)?

@andreasabel
Copy link
Member

Looks like Agda.Interaction.FindFile already has the right logic. Should this ifdef:

#ifdef mingw32_HOST_OS
doesFileExistCaseSensitive f = do
doesFileExist f `and2M` do
bracket (findFirstFile f) (findClose . fst) $
fmap (takeFileName f ==) . getFindDataFileName . snd
#else
doesFileExistCaseSensitive = doesFileExist
#endif

also be activated for mac?

Trying to remember my thought process here. OK:

  • For one, fixing doesFileExistCaseSensitive according to the compile-time OS (rather than the runtime OS) is a hack in the first place (think cross-compilation), but well.
  • But then, case-sensitivity is not really an OS-specific thing, but a it is file-system specific, if I am not mistaken. You can access a case-insensitive system also from Linux.
  • Finally, the find API is different for Window and Unix (BSD/Linux). For Windows, we can ask to find all files matching a pattern, like dir/file.agda, whereas in Unix we only get to ask for the contents of dir/ search through them. This can be costly for large directories, and I did not want to have this running on every call to doesFileExistsCaseSensitive.

It would be optimal if there was a function that would tell you the case-sensitivity of a location (like dir/). In the testsuite, I have some code that infers the case-sensitivity by experiment. Not a hack to be proud of, but at least it should be correct:

agda/test/Fail/Tests.hs

Lines 93 to 106 in 719ba66

-- | A test for case-insensitivity of the file system.
caseInsensitiveFileSystem :: IO Bool
caseInsensitiveFileSystem = fst <$> caseInsensitiveFileSystem4671
-- | A test for case-insensitivity of the file system, using data of test 4671.
caseInsensitiveFileSystem4671 :: IO (Bool, FilePath)
caseInsensitiveFileSystem4671 = do
b <- doesFileExist goldenFileInsens'
return (b, if b then goldenFileInsens else goldenFileSens)
where
dir = testDir </> "customised"
goldenFileSens = dir </> "Issue4671.err.case-sensitive"
goldenFileInsens = dir </> "Issue4671.err.case-insensitive"
goldenFileInsens' = dir </> "Issue4671.err.cAsE-inSensitive" -- case variant, to test file system

@nad
Copy link
Contributor

nad commented Jan 27, 2022

Apparently some recent Linux kernel supports file systems for which floß and FLOSS refer to the same file.

What is the goal with Agda's support for case-insensitive file systems? One goal is presumably to avoid making users confused. Should we also aim to ensure maximum portability? In that case we should perhaps refuse having (say) both A.agda and a.agda on a case-sensitive file system. However, should we then also refuse having both floß.agda and FLOSS.agda? The rules of the file system I mentioned above supposedly depend on the version of Unicode that the file system uses.

@andreasabel
Copy link
Member

In that case we should perhaps refuse having (say) both A.agda and a.agda on a case-sensitive file system.

Yes! I definitely support this.

However, should we then also refuse having both floß.agda and FLOSS.agda?

That sounds too complicated.
If you are using non-ASCII Unicode characters in your filenames, you have to be prepared for trouble.

I'd stick to the common case of upper/lowercase confusions.

@nad
Copy link
Contributor

nad commented Jan 27, 2022

That sounds too complicated.

ghci> :set -XOverloadedStrings 
ghci> :set -package text-icu
package flags have changed, resetting and loading new packages...
ghci> import Data.Text.ICU
ghci> toCaseFold False "FLOSS" == toCaseFold False "floß"
True

@nad
Copy link
Contributor

nad commented Jan 27, 2022

Perhaps one should first normalise:

ghci> "ö" == "ö"
False
ghci> toCaseFold False "ö" == toCaseFold False "ö"
False
ghci> normalize NFD "ö" == normalize NFD "ö"
True

@andreasabel
Copy link
Member

This suggests that text-icu provides the standard. I can change my mind, knowing a standard exists...

@andreasabel
Copy link
Member

Ping @andrejbauer for a self-contained reproducer.

1 similar comment
@andreasabel
Copy link
Member

Ping @andrejbauer for a self-contained reproducer.

@andreasabel andreasabel modified the milestones: 2.6.3, later Aug 31, 2022
@nad nad changed the title Agda asked me to report this bug Handle case-insensitive file systems in a better way Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filesystem case-sensitivity Concerning case-(in)sensitivity of file system platform:macOS Any issue specific to macOS status: info-needed More information is needed from the bug reporter to confirm the issue.
Projects
None yet
Development

No branches or pull requests

6 participants