Skip to content
This repository was archived by the owner on Oct 7, 2020. It is now read-only.

Add Shakefile as possible build script #991

Merged
merged 25 commits into from
Dec 29, 2018
Merged

Add Shakefile as possible build script #991

merged 25 commits into from
Dec 29, 2018

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Dec 11, 2018

Closes #650
I implemented the Shakefile to be run directly from stack.

# one-stop-shop build everything
stack Shakefile.hs build-all

Features:

  • stack is the only build requirement
  • shake is used instead of make
  • should work on all platforms (not fully tested yet)
  • dist target works on windows out of the box, making it easier to create releases for windows
  • submodules are synced / updated at the start -> works after clean clone of project

Quirks:

  • shake.yaml is necessary to start the stack Shakefile.hs ... command before submodules are loaded.
  • On first execution, a compiler is downloaded. However, no compiler is downloaded more than once.

Tested on:

  • Fedora
  • Arch Linux
  • Windows
  • MacOS
  • NixOS

Working on NixOS with the following commands:

nix-shell -p "haskellPackages.ghcWithPackages (pkgs: with pkgs; [zlib tar shake])" zlib
export PATH=$NIX_USER_PROFILE_DIR/profile/bin:$PATH && ghc -O2 ./Shakefile.hs && ./Shakefile build

However, the ghc versions 8.2.1 and 8.4.2 are not easily accessible and must be removed from the list of hieVersions in Shakefile.hs in order to make it work.

@Anrock
Copy link
Collaborator

Anrock commented Dec 12, 2018

I've noticed there is a downside in requirement to download 8.6.2 to run shake script despite me only wanting to check help and build 8.4.4. Not sure if it's fixable.

@lorenzo
Copy link
Collaborator

lorenzo commented Dec 12, 2018

Yeah, having to download the compiler and dependencies to actually install something else is annoying.

@Anrock
Copy link
Collaborator

Anrock commented Dec 12, 2018

I wonder if it's possible to get "whatever ghc is already available" from stack...

@lorenzo
Copy link
Collaborator

lorenzo commented Dec 12, 2018

Or at least the ghc I'm intending to use anyway

@samuelpilz
Copy link
Contributor

samuelpilz commented Dec 12, 2018

@lorenzo @Anrock It is intended that the same resolver as in the normal stack.yaml is used. Therefore, nothing extra is downloaded.
The initial download of the resolver and the ghc is necessary to eliminate all exterior build dependencies beside stack, which is a major benefit on non-linux platforms.

@Anrock
Copy link
Collaborator

Anrock commented Dec 12, 2018

@power-fungus Can you clarify?

Suppose i have clean stack install without any ghc downloaded yet.

If i run stack Shakefile ... then stack uses resolver from shake.yaml with ghc-8.6.2 to run Shakefile.hs itself. So ghc-8.6.2 is downloaded in order to run Shakefile in addition to whatever ghc i need for building hie.

If i run make hie-8.4.4 it executes stack --stack-yaml stack-8.4.4.yaml ..., so stack downloads ghc-8.4.4 and nothing else.

Where does stack.yaml come in in case with Makefile?

@fendor
Copy link
Collaborator Author

fendor commented Dec 12, 2018

@Anrock make hie-8.4.4 still requires the cabal target, which installs cabal with the compiler that is in the local stack.yaml, which currently is ghc-8.6.2.

@Anrock
Copy link
Collaborator

Anrock commented Dec 12, 2018

@fendor i see now! I thought it was just overlooked.

@fendor
Copy link
Collaborator Author

fendor commented Dec 12, 2018

I implemented the dist target in a platform independent way, e.g. tar package and zlib package.
I assume that we can use zlib, since git also requires libz library and should be present on the system, anyways.
However, I have to test that first.

Tested on:

  • Arch Linux
  • Fedora
  • Windows
  • MacOs

@fendor
Copy link
Collaborator Author

fendor commented Dec 13, 2018

I added the icu-macos-fix target.
It would be nice if someone can validate that the Shakefile.hs also works on MacOs.

@fendor
Copy link
Collaborator Author

fendor commented Dec 14, 2018

The pull request should be ready for reviews now.

@Anrock
Copy link
Collaborator

Anrock commented Dec 17, 2018

Just built hie-8.4.4 on OSX with Shakefile. Looks like it working.

@alanz
Copy link
Collaborator

alanz commented Dec 21, 2018

Also, it builds all the individual hie versions, and installs them as ~/.local/bin/hie. Then at the end it copies the last one (for GHC 8.2.1) as ~/.local/bin/hie-xxx, so they all end up being for the same GHC.

@fendor
Copy link
Collaborator Author

fendor commented Dec 21, 2018

I agree it should have a default, the help message seems like a good idea.

I have to check that. In my opinion, the last ghc should be 8.6.2, but I might be mistaken.

@alanz
Copy link
Collaborator

alanz commented Dec 21, 2018 via email

@fendor
Copy link
Collaborator Author

fendor commented Dec 21, 2018

If I understand you correctly, this behaviour should be similar to the Makefile.

cp '$(STACKLOCALBINDIR)/hie' '$(STACKLOCALBINDIR)/hie-$*'

We could copy it directly from the compile path, just like when executing the dist target.

EDIT: just realised what you mean

@fendor
Copy link
Collaborator Author

fendor commented Dec 21, 2018

@alanz The issue about the hie-versions not properly being copied should be resolved.

@alanz
Copy link
Collaborator

alanz commented Dec 22, 2018

LGTM

@alanz
Copy link
Collaborator

alanz commented Dec 22, 2018

One thing though, current master builds for 8.6.3 too.

@fendor
Copy link
Collaborator Author

fendor commented Dec 22, 2018

I will update the pull request to also include 8.6.3.
I would also update the Readme to use the Shakefile.

@alanz alanz added this to the 2018-12 milestone Dec 24, 2018
shake.yaml Outdated
@@ -0,0 +1,7 @@
# Used to provide a different environment for the shake build script
resolver: nightly-2018-12-01 # GHC 8.6.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any way we can do something like "use the system resolver on this machine"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As far as I can tell, since there is a stack.yaml in the project, the script will always default to that.

However,one can work around this by compiling the shakefile.

Copy link
Contributor

Choose a reason for hiding this comment

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

An extra stack.yaml is required since the other yamls contain information about extra dependencies in the submodules folder, which is not present on a non-recursive clone or if the submodules change. Since the build script should take care of updating the git submodules, they are not allowed to be referenced within the stack-yaml that is used by Shakefile.hs.

It is possible, however, to include system-ghc: true within the shake.yaml (see option) However, this seems to be incompatible with Docker and nix.

@@ -170,7 +176,7 @@ helpMessage = do
out ""
out "Targets:"
out
" build Builds hie for all supported GHC versions (8.2.1, 8.2.2, 8.4.2, 8.4.3, 8.4.4, 8.6.1 and 8.6.2)"
" build Builds hie for all supported GHC versions (8.2.1, 8.2.2, 8.4.2, 8.4.3, 8.4.4, 8.6.1, 8.6.2 and 8.6.3)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should eventually refactor this (and the next set of targets) to be generated from the list of hieVersions, so we can set that in one place only.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in the next version of the shakefile i would like to refactor this to obtain the list of hie-versions from present stack.yaml files in the directory. But i felt like this should be in another pull request.

@alanz
Copy link
Collaborator

alanz commented Dec 27, 2018

Is this ready for merge yet?

@fendor
Copy link
Collaborator Author

fendor commented Dec 27, 2018

Technically yes, but the README.md needs updates that one should use ./Shakefile.hs. This should be done tomorrow.
Also, it is not clear to me if we should use system-ghc: true in shake.yaml now. I would like to test it tomorrow.
Moreover, should the Makefile and BuilAll.ps1 be deleted in this pull request to avoid confusion?

@alanz
Copy link
Collaborator

alanz commented Dec 27, 2018

I would say we should add it, and keep the existing Makefile and .ps1 file.

And add a section saying we now have experimental Shake build support, and to please try it and report.

Then we can see what kind of feedback we get.

@fendor
Copy link
Collaborator Author

fendor commented Dec 27, 2018

Ok, then I will revert the changes to the README.md and add a section for the Shakefile.hs.

@fendor
Copy link
Collaborator Author

fendor commented Dec 28, 2018

README is updated, but I did not manage to use the option system-ghc: true correctly.

I think, it is ready to merge now.

@alanz
Copy link
Collaborator

alanz commented Dec 28, 2018

Given #1008, I wonder if we should not use an LTS that is not for GHC 8.6.3.

Otherwise it looks good. I think the build is failing because hackage was being restarted.

@fendor
Copy link
Collaborator Author

fendor commented Dec 28, 2018

Makes sense, LTS-12.25 looks fine?

@fendor
Copy link
Collaborator Author

fendor commented Dec 28, 2018

LTS has been updated

@alanz alanz merged commit 03dcb8a into haskell:master Dec 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants