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

Fix build with cabal #3722

Merged
merged 4 commits into from
May 31, 2023
Merged

Fix build with cabal #3722

merged 4 commits into from
May 31, 2023

Conversation

jneira
Copy link
Contributor

@jneira jneira commented Jan 1, 2023

The pr updates cabal.project to adjust it to last structural changes.
Unfortunately the codebase now uses th referencing files with relative paths and stack and cabal sets a different cwd 😟 (haskell/haskell-language-server#481)
Not sure how coud we solve that behaviour tbh. I believe atleats stack sets some C defs and we could add CPP conditions but i don't like it at all.

Another caveat: the haskeline commit 2944b11d19ee034c48276edc991736105c9d6143 does not exist anymore.
It is being referenced in stack.yaml but i am not sure how is even working. I've replicated the commit in my repo, but obviously it cant be the definitive solution.

The cabal build should be checked in ci to keep it sync (even with an optional job)

@jneira
Copy link
Contributor Author

jneira commented Jan 3, 2023

Unfortunately the codebase now uses th referencing files with relative paths and stack and cabal sets a different cwd 😟 (haskell/haskell-language-server#481)

I expected ci should fail due to changes in Query.hs 🤔

Copy link
Contributor

@aryairani aryairani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @jneira — we don't currently support building with cabal beyond what's in contrib/.

However, we might support cabal in the future, so I'm averse to adding cabal.project to .gitignore.

Not sure how coud we solve that behaviour tbh.

We use symlinks (sql/) because stack and haskell-language-server already use different cwds. We use more symlinks (unison/) because we have another project that uses unison as a submodule under the unison/ path.

Maybe adding more symlinks will work for you, but for the submodule reason above, we don't want to change Query.hs

If you want to merge just the contrib/cabal.project changes, possibly another symlink, and some development.markdown changes, that would be okay.

@jneira
Copy link
Contributor Author

jneira commented Jan 7, 2023

Thanks for the feedback: sure, will revert the .gitignore, great to know cabal could be a first citizen. 😄
hls support for cabal is better than for stack and that is a good argument to support cabal imo (the reason hls uses the same cwd as cabal is #481)

Maybe adding more symlinks will work for you, but for the submodule reason above, we don't want to change Query.hs

Sure, my fault, i did not check lastest instructions for building in windows and i did not activate symlinking. Query.hsworks with any change after activate them.

I am about to upload last versions with the needed changes

@jneira
Copy link
Contributor Author

jneira commented Jan 7, 2023

@aryairani changes done, still left the issue with the haskeline commit used in stack.yaml

The commit does not belong to any branch: haskell/haskeline@d6c2643

And cabal throw an error when trying to fetch it:

fatal: Could not parse object 'd6c2643b0d5c19be7e440615c6f84d603d4bc648'.

(it works for non orphan commits)

Should be cherry-pick the commit to the unison haskeline fork? I think it would be good to not depend on an orphaned commit of a external repo

@aryairani
Copy link
Contributor

Hi @jneira sorry I missed your last messages. The haskeline issue should be resolved now if you want to try again 😬

@dolio
Copy link
Contributor

dolio commented May 31, 2023

This looks good to me. It closely matches what I use locally.

Although, I don't even bother with a custom haskelline/configurator commit. I just use something from hackage and haven't noticed problems.

@aryairani aryairani merged commit 072f3a3 into unisonweb:trunk May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants