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

Rework Makefile #684

Closed
sh0rez opened this issue Jun 19, 2019 · 6 comments
Closed

Rework Makefile #684

sh0rez opened this issue Jun 19, 2019 · 6 comments
Labels
component/packaging type/feature Something new we should do

Comments

@sh0rez
Copy link
Member

sh0rez commented Jun 19, 2019

Is your feature request related to a problem? Please describe.
The current Makefile is based on the one cortex uses, but does not fit this project very well IMHO.

In comparison to e.g. Go, Makefiles are very hard to read and debug. The automatic discovery of binaries implemented using find, etc. makes it even worse and according to @slim-bean it may be replaced in favor of static targets.

Describe the solution you'd like
We should definitely clean-up / rewrite the Makefile or even the whole build process. Currently, I see multiple options to choose from:

The current way: Makefile

We certainly could rewrite / clean-up the current Makefile to make it more readable and understandable.
This would involve checking everything that is in it is even required, including:

  1. .uptodate and BUILD_IN_DOCKER: The end user always needs to create a file to prevent the docker container from being run. This is not documented and not intuitive. And not even CircleCI seems to use the container, so does anyone need this?
  2. Generating protobuf files inside of vendor!? Of course cortex is in there, but this looks very ugly. Are there better solutions?
  3. Yacc: What is yacc even doing here? Please verify we need it.
  4. Binary discovery: Currently, we traverse the tree to find all applications we can possibly build. I vote for replacing this with static targets
  5. docker run workarounds for CircleCI and Cloud Builder .. we do not use docker in CircleCI and do we even use Cloud Builder?
  6. Docker image building: Needs to be rewritten anyways, as BuildKit and experimental CLI features wil be required by feat(docker): multi-arch Dockerfile #668.
  7. Releasing: currently broken anyways, will probably be replaced by GoReleaser, in case it fits our needs (working on it)

Pros:

  • We already have a Makefile, we could try to preserve the workflow
  • make is available nearly everywhere

Cons:

  • This is a huge bunch of open points, and to be honest I don't believe the Makefile will ever get nice and tidy.
  • Makefiles are hard.

The fancy way: Magefile

There is another project around, promising to unite the best of Go and make, called mage. It essentially allows writing make targets as regular go functions and execute them without having to worry about building, etc.

Pros:

  • All the power of Go (yee!)
  • People around should know Go (duh)
  • Nice library for executing shell commands, etc.
  • May be split into multiple .go files, keeps it tidy (one for compiling, one for releasing, one for helm, etc.)

Cons:

The hands-dirty way:

Write our own build.go file, with build-constrains which runs every required command using cmd.Run, uses some cli library for subcommands and get's the job done.

Pros:

  • Power of Magefile (minus comfort)
  • No additional tools needed

Cons:

  • we are getting
  • our hands
  • dirty

To sum up, I vote for Magefiles. Thoughts on this?

@slim-bean
Copy link
Collaborator

+1 for Mage

@tomwilkie @gouthamve @Kuqd @steven-sheehy thoughts? this would divert us from cortex/prometheus path quite a bit which are using traditional make files.

But personally I have no love for makefiles and would be happy to see a go based solution, especially one that keeps the semantics of make: i.e. mage all could still be a target.

Or honestly we could keep make around to install mage and have a make all execute mage

@steven-sheehy
Copy link
Contributor

I think the Makefile should be kept for now and Mage can be added as an exploratory effort at the same time without affecting the CircleCI builds. I don't think the switchover to Mage should be done until after a 1.0 release, in my opinion.

@cyriltovena
Copy link
Contributor

It looks nice, but I like the docker build image too.

I usually enjoy makefile but that one is definitely hard to maintain.

I might be fully convinced by seeing the final results, so have some fun at it and let us know.

@daixiang0
Copy link
Contributor

I think the Makefile shoule be kept too, mage is still newer than make, maybe there are some potential issues in mage, while a go based solution is so cool.

@sh0rez sh0rez mentioned this issue Jun 20, 2019
49 tasks
@sh0rez
Copy link
Member Author

sh0rez commented Jun 20, 2019

Okay, I've sketched out a very basic Magefile in #687 for everyone who wants to try it. Still in a very early stage, but it should give an idea how it is gonna work.

Feedback is more than welcome!

@cyriltovena cyriltovena added component/packaging type/feature Something new we should do labels Jun 20, 2019
@slim-bean
Copy link
Collaborator

@sh0rez with the changes from #753 most of the pain points you mentioned were simplified with the exception of release, we have another issue #659 related to releasing, I think we are ok to close this issue and continue the discussion about release work there?

@sh0rez sh0rez closed this as completed Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/packaging type/feature Something new we should do
Projects
None yet
Development

No branches or pull requests

5 participants