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

Support for asm-sources, cmm-sources, and js-sources in executable components #8639

Closed
hsyl20 opened this issue Dec 20, 2022 · 4 comments
Closed

Comments

@hsyl20
Copy link
Collaborator

hsyl20 commented Dec 20, 2022

Describe the bug

Cabal doesn't support asm-sources, cmm-sources, and js-sources in executable components.

To Reproduce

$ mkdir cabal-foreign
$ cd cabal-foreign
$ cabal init --cabal-version=3.0
$ cabal build ; works fine
$ echo "    asm-sources: whatever.s" >> cabal-foreign.cabal
$ echo "    js-sources: whatever.js" >> cabal-foreign.cabal
$ echo "    cmm-sources: whatever.cmm" >> cabal-foreign.cabal
$ cabal clean && cabal build ; should fail because of the missing files, but doesn't

Expected behavior

Cabal should take into account the specified foreign files: link them when present, or fail when not present as in the reproducer above.

At the very least a warning that the fields are parsed but unused would be a better UX.

System information

  • Linux
  • cabal-install 3.6.2.0 and HEAD
  • ghc 9.2.4

Additional context

I've noticed this bug while I was adapting the implementation of asm-sources to fix the support for js-sources in #8636.

As far as I can tell, the issue is in Distribution.Simple.GHC module. gbuildSources only returns C and C++ foreign sources (cf fields of BuildSources record). It should also return Cmm, asm, and js sources. Then BuildSources new fields must be handled properly in gbuild.

It looks like the code to build C/C++ sources for a library (in buildOrReplLib) is duplicated in gbuild to build an executable. I was reluctant to copy-paste the code handling asm/cmm/js sources from buildOrReplLib to gbuild too. It should probably be factorized instead. That's why I didn't do it as part of #8636.

@chreekat
Copy link
Collaborator

I just ran into this :D I'll see what I can slap together during Zurihac.

mergify bot pushed a commit that referenced this issue Jul 16, 2023
* WIP support asm/cmm/js sources in executable components (#8639)

* Factorise extra src code for lib/exe and add extra exe src tests

* Add extra sources to linking step

* lint

* lint

* Don't build js sources for executables on non-js hosts

* Fix cabal.out for CmmSourcesExe test and lint

* Update changelog

* Slight changes
mergify bot pushed a commit that referenced this issue Jul 16, 2023
* WIP support asm/cmm/js sources in executable components (#8639)

* Factorise extra src code for lib/exe and add extra exe src tests

* Add extra sources to linking step

* lint

* lint

* Don't build js sources for executables on non-js hosts

* Fix cabal.out for CmmSourcesExe test and lint

* Update changelog

* Slight changes

(cherry picked from commit 3350a6c)

# Conflicts:
#	Cabal/src/Distribution/Simple/GHC.hs
@mpickering
Copy link
Collaborator

It's pretty unclear why buildAndReplLib and gbuild are separate functions (for reasons like this).

@ysangkok
Copy link
Member

can this be closed now since #9061 was merged?

@hsyl20
Copy link
Collaborator Author

hsyl20 commented May 21, 2024

Indeed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants