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

cabal-gild-1.3.2 breaks discover src #89

Closed
michaelpj opened this issue Jun 20, 2024 · 13 comments · Fixed by #92
Closed

cabal-gild-1.3.2 breaks discover src #89

michaelpj opened this issue Jun 20, 2024 · 13 comments · Fixed by #92
Labels
bug Something isn't working

Comments

@michaelpj
Copy link

Not sure what's going on, but an index-state bump allowed HLS to pull in newer cabal-gild, which broke our tests. I manually tracked it down to cabal-gild-1.3.2, so I guess #84.

Here's what we have in the failing test, I don't know why this would have started failing now? https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-cabal-gild-plugin/test/testdata/commented_testdata.cabal#L8

@michaelpj
Copy link
Author

I can't make it work at all with newer cabal-gild.

@michaelpj
Copy link
Author

Perhaps something changed to be sensitive to the working directory?

@tfausak
Copy link
Owner

tfausak commented Jun 20, 2024

Hmm, it works as expected for me with both 1.3.2.0 and 1.4.0.0. I did change how discovery works behind the scenes, but if you're not passing --include or --exclude then it should work the same as before. How exactly is cabal-gild being invoked?

Here's an example showing it working:

# cat example.cabal
-- cabal-gild: discover src
exposed-modules: whatever

# find . -type f
./example.cabal
./src/M.hs
./src/N.hs

# cabal-gild --version
1.4.0.0

# cabal-gild -i example.cabal
-- cabal-gild: discover src
exposed-modules:
  M
  N

It also works when called from another directory:

# cabal-gild -i gh-89/example.cabal
-- cabal-gild: discover src
exposed-modules:
  M
  N

And when using --stdin:

# cabal-gild --stdin example.cabal < example.cabal
-- cabal-gild: discover src
exposed-modules:
  M
  N

And with --stdin from another directory:

# cabal-gild --stdin gh-89/example.cabal < gh-89/example.cabal
-- cabal-gild: discover src
exposed-modules:
  M
  N

@tfausak
Copy link
Owner

tfausak commented Jun 20, 2024

It looks like your test failed on Ubuntu but succeeded on Windows: https://github.com/haskell/haskell-language-server/actions/runs/9594229151/job/26456359466#step:34:42

I'm not sure what to make of that. Maybe some directory or file casing is weird somewhere?

@tfausak
Copy link
Owner

tfausak commented Jun 25, 2024

@michaelpj can you provide more detail here? I wasn't able to reproduce this problem.

@michaelpj
Copy link
Author

Sorry , I'm on holiday at the moment, but this is on my list to get back to. Possibly @fendor is also interested?

@fendor
Copy link

fendor commented Jun 26, 2024

I can reproduce on linux:

> cabal install -z cabal-gild --constraints="cabal-gild ==1.3.2.0"
> cd <hls>
> cd plugins/hls-cabal-gild-plugin/test/testdata/
> cabal-gild --stdin=/home/hugin/Documents/haskell/hls/plugins/hls-cabal-gild-plugin/test/testdata/commented_testdata.cabal --input=- < commented_testdata.cabal 
cabal-version: 2.4
name: testdata
version: 0.1.0.0
author: Banana
extra-source-files: CHANGELOG.md

library
  -- cabal-gild: discover src
  exposed-modules:
  build-depends: base ^>=4.14.1.0
  hs-source-dirs: src
  default-language: Haskell2010

Does this help?

The absolute path to --stdin seems to be a problem, if I just pass in commented_testdata.cabal, it works fine!

@tfausak
Copy link
Owner

tfausak commented Jun 26, 2024

Ah, that's the ticket! Thank you!

If you pass an absolute path to either --input or --stdin, then Gild won't discover modules.

# find . -type f
./example.cabal
./M.hs

# cat example.cabal 
-- cabal-gild: discover .
exposed-modules: whatever

# cabal-gild --version
1.3.2.0

# cabal-gild --input example.cabal 
-- cabal-gild: discover .
exposed-modules: M

# cabal-gild --input $PWD/example.cabal
-- cabal-gild: discover .
exposed-modules:

@tfausak
Copy link
Owner

tfausak commented Jun 26, 2024

Ah, I think I've found the problem:

files <- Trans.lift $ MonadWalk.walk "." inclusions exclusions

Gild always searches from the current directory. The inclusions are patterns of files to discover. By default it's basically $ROOT/**. When using relative paths, $ROOT is often ., which is fine. However if you use an absolute path, then $ROOT would be that path, like /foo. Even if you're in /foo, the inclusion will fail because it's essentially just a string match — it doesn't resolve paths to see if they're the same.

I think this can be fixed by using makeRelative in a few key places.

@fendor
Copy link

fendor commented Jun 26, 2024

I would also be trivial to pass in the relative location in HLS if you think this is a user error on the HLS side.

@michaelpj
Copy link
Author

I think it's reasonable to expect that tools like this should work the same with relative and absolute paths, even if humans are more likely to use relative ones.

@tfausak
Copy link
Owner

tfausak commented Jun 28, 2024

I agree. It's a bug that Gild's discovery doesn't work with absolute paths.

@tfausak
Copy link
Owner

tfausak commented Jun 29, 2024

Should be fixed in 1.4.0.1! Let me know if you want a backport to 1.3.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants