build: improve the clean target

[orbea]

• Uses a sentinel file for the obj directory.
• Removes the binary with make clean.
• Uses rm -f to ensure the clean target always succeeds.

make(1) has difficulty depending on directories which is where a sentinel file help.

[Alcaro]

I kinda like how make clean keeps the dir, makes things jump around less if a graphical file browser is open in that dir. For the same reason, I'm not a huge fan of deleting the target file.

I agree that depending on a directory's timestamp is troublesome, but I haven't seen any problems with depending on directories existing (that | thing). Please clarify what difficulties you're thinking of.

I'm not sure why you're making the object file directory into a variable. If you want it variable between targets, I'd rather see some extra string added to the object files' names.

The ||true is because if the object file directory is empty, obj/* expands to the empty set, and rm with no filenames fails, even with -f. Easy to fix if some of your other proposed changes are included, but only if we can reach an agreement on them.

[orbea]

I kinda like how make clean keeps the dir, makes things jump around less if a graphical file browser is open in that dir. For the same reason, I'm not a huge fan of deleting the target file.

I don't use graphical file browsers very often so I did not consider that. However generally speaking a clean target is intended to remove files and directories created during the build process returning the source tree to a "clean" state.

In regards of the target file, I was having this problem.

$ make
....
# make changes
$ make clean && make
make: Nothing to be done for 'all'.

This forces me to use git clean -xdf or manually remove the file instead.

I agree that depending on a directory's timestamp is troublesome, but I haven't seen any problems with depending on directories existing (that | thing). Please clarify what difficulties you're thinking of.

This is generally a problem during parallel make and the current solution is to use order-only-prerequisites. However that will result in the directory being created every time make is run since make is not capable of using a directory as a prerequisite while a sentinel file will avoid that since the obj/.tag file exists. So my opinion is that a sentinel file is a cleaner and more robust solution.

I'm not sure why you're making the object file directory into a variable. If you want it variable between targets, I'd rather see some extra string added to the object files' names.

I found it easier if the directory name was controlled in one place instead of being hard-coded in multiple places, but this is not very important and whatever you prefer will be fine for me.

The ||true is because if the object file directory is empty, obj/* expands to the empty set, and rm with no filenames fails, even with -f. Easy to fix if some of your other proposed changes are included, but only if we can reach an agreement on them.

I found this strange:

$ make clean
mkdir obj
rm obj/* || true
rm: cannot remove 'obj/*': No such file or directory

Where rm -f is what Makefiles often use for clean rules since they are expected to work even if the files don't exist. And with a sentinel file the obj directory is only created if its really needed and not implicitly as part of the clean rule.

[Alcaro]

However generally speaking a clean target is intended to remove files and directories created during the build process

Yeah, opinions on build systems vary as much as opinions on code formatting. I've never put much stock in doing things just because everyone else does it; if it's a good idea, I want to know why (and then I'll be happy to do it), and if it's just cargo culting, I'd rather not.

# make changes
make: Nothing to be done for 'all'.

If the build target isn't remade after changing something, that's a bug in the dependency list, not the make clean target. Did you change a .h and not any .cpp, or something?

This project is a bit special in that it doesn't have a separate linking step - it just goes straight from source to executable. It's not even guaranteed to create the obj dir. Throws plenty of traditional wisdom out the window.

However that will result in the directory being created every time make is run

I've never seen that happen. If it did, it'd error out, mkdir doesn't want to create a directory that already exists. Are you using a non-GNU make or something?

(Also if we need a tag file, I'd prefer echo Signature: 8a477f597d28d172789f06886806bc55 > obj/CACHEDIR.TAG.)

I found it easier if the directory name was controlled in one place instead of being hard-coded in multiple places

So just an organizational thing? Sure, sounds legit. Though the dir is also named in the build-*.sh scripts, to put the PGO stuff there, so we can't make it configurable.

rm: cannot remove 'obj/*': No such file or directory

Okay, so make keeps globs unchanged if the target doesn't exist. Guess I misremembered something. Then changing that to -f would be an improvement.

I'm approving the following changes:

• Remove |obj from make clean target
• Swap ||true for -f
• Add headers to the build dependencies
• Make the obj dir's name a variable, but with no supported way to override

but I'm waiting with them until we reach agreements on everything.

[orbea]

However generally speaking a clean target is intended to remove files and directories created during the build process

Yeah, opinions on build systems vary as much as opinions on code formatting. I've never put much stock in doing things just because everyone else does it; if it's a good idea, I want to know why (and then I'll be happy to do it), and if it's just cargo culting, I'd rather not.

While your point is entirely valid, in this case I do think the conventional practice has been like this for a long time for a good reason.

If the build target isn't remade after changing something, that's a bug in the dependency list, not the make clean target. Did you change a .h and not any .cpp, or something?

In this case I changed the Makefile so I don't think adding prerequisites will be enough.

This project is a bit special in that it doesn't have a separate linking step - it just goes straight from source to executable. It's not even guaranteed to create the obj dir. Throws plenty of traditional wisdom out the window.

I was considering if this could be changed to allow building the object files in parallel and avoid building empty translation units (Which I believe is a -pedantic warning with clang), but regardless this is out of scope of the current PR.

However that will result in the directory being created every time make is run

I've never seen that happen. If it did, it'd error out, mkdir doesn't want to create a directory that already exists. Are you using a non-GNU make or something?

You are right, its not mkdir as that would print mkdir: cannot create directory ‘obj’: File exists, maybe gmake itself changed at some point? When I last had this issue in other projects it was years ago.

$ make clean
mkdir obj
rm obj/* || true
rm: cannot remove 'obj/*': No such file or directory

$ make clean
rm obj/* || true
rm: cannot remove 'obj/*': No such file or directory

(Also if we need a tag file, I'd prefer echo Signature: 8a477f597d28d172789f06886806bc55 > obj/CACHEDIR.TAG.)

I found it easier if the directory name was controlled in one place instead of being hard-coded in multiple places

So just an organizational thing? Sure, sounds legit. Though the dir is also named in the build-*.sh scripts, to put the PGO stuff there, so we can't make it configurable.

There is always override if you don't want a variable changed, but I thought it was overkill in this case. Regardless it doesn't really matter to me if its a variable or not.

rm: cannot remove 'obj/*': No such file or directory

Okay, so make keeps globs unchanged if the target doesn't exist. Guess I misremembered something. Then changing that to -f would be an improvement.

I'm approving the following changes:

* Remove |obj from make clean target

* Swap ||true for -f

* Add headers to the build dependencies

* Make the obj dir's name a variable, but with no supported way to override

but I'm waiting with them until we reach agreements on everything.

Its your project so whatever you prefer is fine, My intent is to improve the build system enough that I can try adding Flips to the Gentoo repo, but whether the Gentoo devs allow that or not I don't know. I will change my PR to the above changes later today probably.

[Alcaro]

Yes, but what is said good reason?

That one's fixable by making the makefile itself a dependency.

Flips is small; debug builds only take a second, and the optimized one spends 80% of its compile time PGOing. In both cases, there are no real benefits to parallelizing it. ...yet, at least. It would be an improvement, but no need to hurry.

Anyone who tries to configure the objdir gets to keep the pieces if/when it blows up. Anything beyond that is safe to ignore.

Sure, sounds good to me. All distros have subtly different procedures, I'd quite like to not have to keep track of all that.