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

Workaround for ghcjs hardcoding to linux #511

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

alexfmpe
Copy link
Member

@alexfmpe alexfmpe commented Jul 24, 2019

Due to ghcjs/ghcjs#674
Upstreaming @vaibhavsagar's fix since we both ran into this on different projects.

Test case: obsidiansystems/obelisk@develop...alexfmpe:ae-error-foundation

else nixpkgs.haskell.lib.overrideCabal package (drv: {
postPatch = (drv.postPatch or "") + nixpkgs.lib.optionalString (system == "x86_64-darwin") ''
substituteInPlace ${cabalFile}.cabal --replace 'if os(linux)' 'if os(linux) && !impl(ghcjs)'
substituteInPlace ${cabalFile}.cabal --replace 'if os(osx)' 'if os(linux) && impl(ghcjs)'
Copy link
Member

Choose a reason for hiding this comment

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

Is this right? if os(osx) gets translated to if os(linux) ...?

Copy link
Member

Choose a reason for hiding this comment

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

If it is right...an explanation is worthy.

Copy link
Member

Choose a reason for hiding this comment

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

Talked to @alexfmpe. So basically this is just a very confusing way of writing it. Because of the hardcoding in GHCJS, os(linux) == true and true && x == x so why are we confounding this substitution?

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want the os(linux) patches to get applied, so we change the os(linux) conditional to only apply when GHCJS isn't being used. Since GHCJS always thinks it's running on Linux, we change the os(osx) patches to os(linux) patches.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

because we don't want to apply the actual os(linux) patches (since we're really on macOS)

Copy link
Contributor

Choose a reason for hiding this comment

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

GHCJS thinks we're on Linux, but that doesn't mean we should apply the Linux patches, instead we need to trick GHCJS into applying the macOS patches

Copy link
Contributor

Choose a reason for hiding this comment

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

so we say "these linux patches are not for you, silly GHCJS, but these other (actually macOS) linux patches are!"

Copy link
Member Author

Choose a reason for hiding this comment

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

Note the replacement only happens if system == "x86_64-darwin"

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense. Thanks.

@ali-abrar
Copy link
Member

ali-abrar commented Jul 24, 2019

As @3noch said, this needs a more informative comment/explanation.

Rather than substitute-in-place, please create a patch file and apply it. Please also document where the upstream PR we're waiting on can be found.

@Ericson2314
Copy link
Member

Ericson2314 commented Jan 13, 2020

ghcjs/ghcjs#702 mentions the issues.

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.

5 participants