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

Regression in HEAD leading to build path leaking into object files (preprocessors) #10242

Closed
mpickering opened this issue Aug 2, 2024 · 9 comments
Labels
priority: high 🔥 regression on master Regression that is unreleased and needs to be fixed before release type: bug

Comments

@mpickering
Copy link
Collaborator

In Cabal HEAD, when hsc2hs is invoked an absolute path is passed which then leaks into the object files due to this path being used in a LINE pragma.

In older versions of Cabal this is a relative path which doesn't include the build directory.

I suspect this is due to work performed by @sheaf so I will coordinate investigations with him to investigate a fix.

@mpickering
Copy link
Collaborator Author

Seems that preprocessFile requires an argument of type Maybe (SymbolicPath CWD (Dir Pkg)) but then the filepath which arrives is "/home/matt/mercury-web-backend/zlib-0.7.1.0/", notably not a relative path from CWD to (Dir Pkg).

@mpickering
Copy link
Collaborator Author

It seems this absolute path originates a long way back in cabal-install, so I am a bit wary of changing it. I will talk with Sam.

@mpickering
Copy link
Collaborator Author

This is the change which seems to fix things for local source packages.

 readSourcePackageLocalDirectory verbosity dir cabalFile = do
   monitorFiles [monitorFileHashed cabalFile]
   root <- askRoot
-  let location = LocalUnpackedPackage (root </> dir)
+  let location = LocalUnpackedPackage dir

@Kleidukos
Copy link
Member

@mpickering If I understand wwell, the dir in question has already been resolved to be an absolute path?

@mpickering
Copy link
Collaborator Author

@Kleidukos 3.12 doesn't have this regression but it's important to fix before 3.14 I believe.

@Kleidukos
Copy link
Member

Quite right indeed.

@ulysses4ever ulysses4ever added the regression on master Regression that is unreleased and needs to be fixed before release label Aug 5, 2024
@Kleidukos
Copy link
Member

@mpickering Have you coordinated with @sheaf for a fix?

@sheaf
Copy link
Collaborator

sheaf commented Sep 2, 2024

Yes, Matthew fixed this in #10256 (now merged).

@sheaf sheaf closed this as completed Sep 2, 2024
@Kleidukos
Copy link
Member

Fantastic, this brightens my day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high 🔥 regression on master Regression that is unreleased and needs to be fixed before release type: bug
Projects
None yet
Development

No branches or pull requests

4 participants