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

Remove Makefile & build-all.ps1 and leave only Shakefile #1033

Closed
6 of 7 tasks
Anrock opened this issue Jan 7, 2019 · 17 comments
Closed
6 of 7 tasks

Remove Makefile & build-all.ps1 and leave only Shakefile #1033

Anrock opened this issue Jan 7, 2019 · 17 comments
Assignees
Labels
build Continuous integration and building
Milestone

Comments

@Anrock
Copy link
Collaborator

Anrock commented Jan 7, 2019

Hey, three build scripts are too much.

Right now I'm making a PR that removes both Makefile and build-all.ps1 and updates README about that (also some DRY cleanups).

Is there any objections or specific blockers to using Shake script as the only build script?

Todo:

@Anrock Anrock self-assigned this Jan 7, 2019
@Anrock
Copy link
Collaborator Author

Anrock commented Jan 7, 2019

@alanz @bubba @fendor

@fendor
Copy link
Collaborator

fendor commented Jan 7, 2019

Following Blockers: Shakefile has the issue that not all supported ghc's are being installed. The list of ghc's is not complete.
Thus, the help message is actually wrong.

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 7, 2019

@fendor it's seems 8.4.2 and 8.2.1 are the only ones that's missing. Would it be enough to just add them to hieVersions list?

@fendor
Copy link
Collaborator

fendor commented Jan 7, 2019

yes that is enough

@lukel97
Copy link
Collaborator

lukel97 commented Jan 8, 2019

Can you check that #1032 is also covered in the Shakefile?

@fendor
Copy link
Collaborator

fendor commented Jan 12, 2019

We should also integrate #1008 into the install.hs, e.g. special handling of hie-8.6.3 on windows.

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 12, 2019

@fendor afaik it isn't handled right now, so it's a separate issue.

@leifmetcalf
Copy link
Contributor

What's the advantage of using shake over a normal haskell script?

@lukel97
Copy link
Collaborator

lukel97 commented Jan 16, 2019

@leifmetcalf being able to define it in terms of targets and build rules means it can work out the dependency graph for each binary and only build what’s needed. And it also provides the command line interface for free

@leifmetcalf
Copy link
Contributor

@bubba The current install.hs is just a bunch of phony rules, though.

@lukel97
Copy link
Collaborator

lukel97 commented Jan 16, 2019

@leifmetcalf that is pretty confusing. I had to take a look at it again, but I think its because the path to the hie-x binaries are buried somewhere inside .stack-work, so the targets can't rely on Shake for detecting if its dirty or not since we don't know the path. But thankfully stack already takes care of not doing redundant rebuilding. The phony rules still provide dependencies though between the different levels of installation that can occur (e.g. build hie for one version or for all versions)

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 21, 2019

@bubba i think this is ready to land as soon as CI tests for install.hs are implemented.

@lukel97
Copy link
Collaborator

lukel97 commented Jan 26, 2019

@Anrock just an update to let you know I'm still working on it! Ran into a bunch of issues with caching, I want to make sure that the shake file CI reuses as much cache as possible to prevent jobs from being bogged down

@alanz alanz removed this from the 2019-01 milestone Feb 2, 2019
@alanz alanz added this to the 2019-02 milestone Feb 2, 2019
@Anrock
Copy link
Collaborator Author

Anrock commented Feb 19, 2019

@bubba how things going? Numerous issues regarding make-shake consistency appeared recently.

@lukel97
Copy link
Collaborator

lukel97 commented Feb 21, 2019

@Anrock sorry for the delay: I hit a couple of walls when trying to merge multiple stack caches on CircleCI. For now I think this PR is fine to merge, and we can create an issue for the CI/testing

@Anrock
Copy link
Collaborator Author

Anrock commented Feb 22, 2019

@bubba ok, thanks. I'll try find some time to recheck what else isn't ported to install.hs and send a PR for review.

@Anrock
Copy link
Collaborator Author

Anrock commented Mar 19, 2019

I consider this as done.
Some work remains to do with CI (tests, maybe some cleanup), but i have no idea how to do it, so leaving it to @bubba

@Anrock Anrock closed this as completed Mar 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
build Continuous integration and building
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants