-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Remove mess of build scripts #123
Comments
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 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.
|
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:
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 |
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.
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 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 The scripts we have in 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 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.
|
FWIW this isn't strictly true you can write:
To define |
I tried it out, I get
|
You didn't show what you actually did so I can't say for sure but perhaps you forgot to declare the
|
@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 Let's fix this before it becomes a massive, unmaintainable mess that no one wants to contribute to. |
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 |
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.
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.
Diverge from what? |
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 I hope we can have the resolve to get this fixed now before this becomes a larger problem. |
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)).
This already works with It's not clear to me how the different approaches would impact the ability to |
Its fairly clear that the direction here is towards a of mess scripts. Closing. |
I agree with @stevvooe, this is exactly replicating the docker/docker build system. |
FWIW I also do not follow this logic. A |
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
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.
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 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. |
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. |
I've noticed the Makefile is littered with scripts like this: https://github.com/docker/cli/blob/master/scripts/build/dynbinary. This both contributes to build slow down and obfuscation.
Let's use the Makefile correctly.
The text was updated successfully, but these errors were encountered: