Remove mess of build scripts about cli HOT 16 CLOSED

You didn't show what you actually did so I can't say for sure but perhaps you forgot to declare the TARGET (since the construction I gave doesn't do that). Here is what I see:

ijc@bokrug:tmp$ cat Makefile 
TARGET: VARIABLE = VALUE
TARGET:
	@echo TARGET = $(VARIABLE)
NOTTARGET:
	@echo NOTTARGET = $(VARIABLE)
ijc@bokrug:tmp$ make TARGET
TARGET = VALUE
ijc@bokrug:tmp$ make NOTTARGET
NOTTARGET =

from cli.

We could remove some .PHONY targets from the makefile by getting a list of .go source files and having targets depend on them. I think that would be an improvement.

I've tried this in the past but it hasn't worked out well for me. It can cause weird corner cases like deletion of .go files not leading to a rebuild, and changes in the build environment (say GOOS/GOARCH) being ignored. It also requires a relatively heavyweight command that crawls the filesystem to run before make can invoke any targets, even unrelated targets.

from cli.

I like the current solution. We (mostly) use the Makefile for what it's designed to do (skip targets that are already fresh).

We could remove some .PHONY targets from the makefile by getting a list of .go source files and having targets depend on them. I think that would be an improvement.

Please provide some more concrete details about why you believe the current solution is slower and obfuscated.

Including more of the details of each step in the Makefile target would not be an improvement.

• bash syntax is much more widely understood than Makefile syntax
• makefiles only support global variables, which make it hard to refactor and cleanup the build system (because everything is coupled to globals)
• conditionals and functions in the Makefile are verbose and hard to read

from cli.

We have a lot of Go projects that can build in a variety of environments without such a production. Here is a short list of problems with this approach:

• For every single Makefile invocation, you now have at least on extra process.
• To understand the Makefile, you need to open other script files. For the most part, these are just go build invocations.
• Extra variables are now hidden away in a .variables file, which includes some of the most important variables, such as LDFLAGS and the input and output. To modify variables, one must descend into the scripts section to find common functionality.
• Makefiles are not supposed to require conditionals and functions but you can get most of this through platform includes.
• Extra scripts hide functionality and common patterns, creating a situation that will lead to more complex build over time.

Right now, the moby build takes something like 45 minutes on a fast machine. It was this kind of script hacking that led to such a complex and slow build. Let's not head down this path for the cli, which should just be a shallow go build.

from cli.

Thank you for elaborating on your concerns. I do share many of these same concerns, and I also felt the pain of the former docker/docker build system. I do not wish to repeat the mistakes.

It was this kind of script hacking that led to such a complex and slow build.

This is not accurate. Let's break this into two issues. First the build speed.

The docker/docker build was trying to use a single Dockerfile for every concern. Any time a package in the early layers of the dockerfile was changed it would result in a very long build time.

This problem has been fixed by using separate Dockerfile for different logical groups of tasks. The cross dockerfile is still quite slow, but at least a change to it doesn't require rebuilding the smaller dockerfiles. This is effectively a problem with docker build, which will eventually be addressed by moby/moby#32550. Using alpine base images also improves the speed a bit.

A second cause was writing too many integration tests when unit test would have been sufficient. This is also being addressed. We have an excellent unit test framework in place, and we can be more strict about what requires a slower integration test.

The second issue was the complexity of the old system. One of the major sources of complexity was the hack/make.sh runner. This script setup a bunch of global state that was required to run any task. This made it extremely difficult to identify the dependencies and logic of a specific task.

The scripts we have in docker/cli do not suffer from the same issue. They include any dependencies explicitly by sourcing them. Moving everything to the Dockerfile would actually re-introduce the same problem caused by hack/make.sh.

Another source of complexity was that the "core logic" of each task was often hidden in shared functions (sourced by hack/make.sh). So trying to trace the operation of a specific task was very difficult. The build system in docker/cli does not have this problem. The "core logic" command line is always visible in the target script. Shared dependencies (so far just variables) are sourced in. But their source is obvious, because they are sourced directly from the target. This results in a few files that look quite similar, but makes it much easier to trace.

It is true you have to look in more than one file, but the file to look in is obvious.

I understand how this new system may resemble the old system in some ways, but I do not believe it suffers from the same problems.

• dependencies should remain explicit
• the behaviour of a specific task should be obvious from the script file which implements the task
• the script for a specific makefile target should have a filename that is easily discoverable from the Makefile

from cli.

makefiles only support global variables, which make it hard to refactor and cleanup the build system (because everything is coupled to globals)

FWIW this isn't strictly true you can write:

TARGET: export VARIABLE = VALUE

To define $(VARIABLE) only for TARGET (which could also be a list of targets I think). I'm not 100% sure but I don't think this is gmake specific.

from cli.

I tried it out, I get

Makefile:7: *** recipe commences before first target.

from cli.

@dnephin No, this doesn't resemble the current build system, but it defines a structure in which in can head in that direction. The first step in heading in that direction is to unnecessarily obfuscate Makefile rules into script files. It allows a level of build obfuscation that will slowly diverge and result in an unmaintainable build. When this happens, it will be require a lot of time that no one wants to spend fixing it.

Let's fix this before it becomes a massive, unmaintainable mess that no one wants to contribute to.

from cli.

No, this doesn't resemble the current build system, but it defines a structure in which in can head in that direction. [an unmaintainable mess that no one wants to contribute to.]

A single massive Makefile for everything is just as likely to lead to the same result. The solution is to participate in PR reviews to prevent this from happening. Now that we have a smaller, and well scoped, repository this is much easier to do.

The first step in heading in that direction is to unnecessarily obfuscate Makefile rules into script files.

There's nothing obfuscated about it. It's better organized and better isolated from unrelated pieces. The makefile "rules" are still in the makefile, but their implementation has been moved into a more appropriate file.

It allows a level of build obfuscation that will slowly diverge

Diverge from what?

from cli.

their implementation has been moved into a more appropriate file.

How is this more appropriate than the Makefile?

Isolation is a not a requirement for the components of a Makefile. To understand what this Makefile does, one has to spelunk through 5 directories and 13 scripts than contain no more than a few lines each. Variables, usually defined at the top of a Makefile, are tucked away in a hidden file. This is the definition of obfuscation.

It would be great to simply go get github.com/docker/cli/cmd/docker` and have a working client. Any room that allows arbitrary build specialization will be another barrier to ensuring that continues to work.

I hope we can have the resolve to get this fixed now before this becomes a larger problem.

from cli.

Isolation is a not a requirement for the components of a Makefile.

That's true, but the goal here is not to write a Makefile, it's to codify developer operations so they are repeatable and convenient.

The one place where it does have value we chose not to use it (as @aaronlehmann mentioned in #123 (comment)).

It would be great to simply go get github.com/docker/cli/cmd/docker and have a working client.

This already works with CGO_ENABLED=0. The gcc requirement has nothing to do with these scripts.

It's not clear to me how the different approaches would impact the ability to go get. Anything that can be added in a script could be added to the Makefile.

from cli.

Its fairly clear that the direction here is towards a of mess scripts. Closing.

from cli.

I agree with @stevvooe, this is exactly replicating the docker/docker build system.
It may be more intelligently executed, but it's just as much of a pain to maintain and discover.

from cli.

Isolation is a not a requirement for the components of a Makefile.

That's true, but the goal here is not to write a Makefile, it's to codify developer operations so they are repeatable and convenient.

FWIW I also do not follow this logic. A Makefile target (or set of targets) is no less "codified" or "repeatable" and they are certainly more "convenient" than digging around through various scripts and includes (the current state, nevermind the possibility of it worsening which @stevvooe brings up).

from cli.

this is exactly replicating the docker/docker build system.

There are definitely some similarities. For one it uses the same tools (make and bash). I would also expect many of the tasks to be similar because it builds part of the same source. I think what's important is that we can keep what works, and fix what doesn't. The big problems with the docker/docker build system were speed and difficulty to trace the operations. This system doesn't suffer from either of those problems. The major differences, as I see them are:

• docker/docker used a .PHONY makefile to build images and create containers. Currently docker/cli does the same thing, but we're discussing changing this in #107. Either way, this similarity doesn't seem to be relevant to this issue.
• docker/docker used a bash script called hack/make.sh as a task runner. This script sourced a bunch of files and set up environment variables and state for the the tasks. docker/cli uses Makefile as a task runner. It contains no state. Each task script has to source its dependencies explicitly.
• docker/docker used a single monolithic Dockerfile which created an image for all developer tasks, which resulted in very slow operations. docker/cli uses smaller Dockerfiles where tasks require different dependencies, so that changes to one to not trigger unnecessary cache misses.

The most relevant to this topic seems to be the second point. Moving everything into the Makefile actually re-introduces some of the problems we had with the old system because the task runner would be responsible for setting up the environment.

but it's just as much of a pain to maintain and discover.

Are you sure this isn't just an initial reaction to something new?

I think there are two types of discovery. For developers looking to run tasks they need to be able to find out what tasks to run. Both implementations are equally discoverable in this regard because both solutions have a list of targets in the Makefile.

What's missing is a description for each task that helps the developer understand what the task does. This is another shortcoming of make. It requires a hacky solution to grep makefiles for special comment lines to provide these descriptions. A tool that was actually designed to solve this problem would provide a place to include descriptions and view them from a list command. This goes back to my argument about keeping the Makefile small, so that we can switch to a more appropriate tool in the near future.

The second case for discovery is when you want to make modifications to a task. In this case there is a clear link from the Makefile to the task script. The filename of the task script is in the Makefile! From the task script all dependencies are sourced explicitly. Yes it does involve opening a couple extra files. I hope everyone is capable of opening a couple files and this isn't a significant barrier. These files should be organized into directories that describe their behaviour (test, build, etc), which makes it easy to remember where to find them. If this isn't the case let's definitely fix that.

from cli.

A Makefile target (or set of targets) is no less "codified" or "repeatable"

I don't think anyone was making this argument (I certainly wasn't). My argument was about properly framing the problem. The goal is not to write a Makefile, it's to provide a convenient way of running development tasks. Makefiles are definitely capable of doing this. The point I'm trying to make is that there are other options. By limiting our exposure to Makefiles we make it easier on ourselves to switch to other (potentially better) options.

from cli.