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

WIP: Remove Makefile & build-all.ps1 #1035

Closed
wants to merge 13 commits into from
Closed

WIP: Remove Makefile & build-all.ps1 #1035

wants to merge 13 commits into from

Conversation

Anrock
Copy link
Collaborator

@Anrock Anrock commented Jan 7, 2019

Closes #1033

TODO:

@alanz
Copy link
Collaborator

alanz commented Jan 8, 2019

I think we should include something in CI that builds using this, preferably on all the platforms we support.

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 8, 2019

I've compared shakefile and makefile and the only notable difference i've found is cabal target. Otherwise they look almost identical.
Makefile:

cabal:
	stack install cabal-install
	cabal update
	cabal install Cabal-2.4.1.0 --with-compiler=$(GHC)

Shakefile:

installCabal :: GhcPath -> Action ()
installCabal ghc = do
  execStack_ ["install", "cabal-install"]
  execCabal_ ["v1-update"]
  execCabal_ ["v1-install", "Cabal-2.4.1.0", "--with-compiler=" ++ ghc]

And i have no idea what's the difference between, for example, cabal install and cabal v1-install. I assume that cabal install is implicitly calls v1-install until v2 commands are default, right? Can someone comment on this?

@bubba can you take CI tests part? I have no experience in that.

@lukel97 lukel97 self-assigned this Jan 9, 2019
@lukel97
Copy link
Collaborator

lukel97 commented Jan 9, 2019

@Anrock Yep! I'll add some basic tests on CircleCI

@alanz
Copy link
Collaborator

alanz commented Jan 9, 2019

I just realised that if we get the the hie-wrapper to suggest an installation, we end up with a message like

"we have detected you need hie-8.4.3, but it is not on your path. Please install by doing ShakeFile hie-8.4.3".

It would be better to rename ShakeFile.hs to something else, like build.hs, make.hs, install.hs.

@alanz
Copy link
Collaborator

alanz commented Jan 9, 2019

@Anrock we took out the v1-prefix on the cabal stuff because some cabal versions for the earlier GHC compilers don't understand them.

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 10, 2019

@bubba thanks!
@alanz so i should remove them from Shakefile as well, right? Regarding renaming - i'll rename it to install.hs then.

@alanz
Copy link
Collaborator

alanz commented Jan 10, 2019

@Anrock yes, I think remove the v1- prefixes, and install.hs sounds fine.

@alanz
Copy link
Collaborator

alanz commented Jan 11, 2019

I see the build failed on circle CI

@lukel97
Copy link
Collaborator

lukel97 commented Jan 11, 2019

Sorry - this is from me trying to add a test on this branch. Feel free to revert the commit and merge, I can open a separate PR for it later today

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 11, 2019

I would rather wait for PR with Shake tests before proceeding with this one.

CI error indicates that it's probably an out of memory error. Can somenone just try restarting it and see if it still fails?

@lukel97
Copy link
Collaborator

lukel97 commented Jan 11, 2019

@Anrock the workspace-sharing setup between jobs isn't working in the shakefile job, causing it to try and rebuild the entire thing from scratch, its most likely caused by me butchering something in the config.yml file

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 11, 2019

@bubba ok. I'll rebase this PR on master throwing out your commits then.

@Anrock
Copy link
Collaborator Author

Anrock commented Jan 11, 2019

I find it easier to implement this via multiple self-contained PRs, so i'm closing this. Todo list copied to #1033.

@Anrock Anrock closed this Jan 11, 2019
@Anrock Anrock deleted the remove-make-ps branch January 11, 2019 16:06
@alanz alanz added this to the 2019-01 milestone Feb 2, 2019
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.

Remove Makefile & build-all.ps1 and leave only Shakefile
3 participants