avoid useless fomantic rebuilds

[silverwind]

the fomantic target for some reason does sometimes not update the timestamp on the destination directory, leading to useless rebuild.

Manually update the timestamp at the end of the build to avoid this and also added the same to js/css targets.

@zeripath see if that helps.

[codecov-io]

Codecov Report

Merging #9999 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #9999      +/-   ##
==========================================
- Coverage   42.26%   42.26%   -0.01%     
==========================================
  Files         610      610              
  Lines       80372    80372              
==========================================
- Hits        33973    33967       -6     
- Misses      42220    42228       +8     
+ Partials     4179     4177       -2

Impacted Files	Coverage Δ
models/unit.go	37.03% <0%> (-2.47%)	⬇️
services/pull/check.go	54.54% <0%> (-2.1%)	⬇️
routers/repo/view.go	38.59% <0%> (-0.88%)	⬇️
modules/log/file.go	77.62% <0%> (+2.09%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd094ee...8b6445e. Read the comment docs.

[zeripath]

Yup seems to fix it.

BTW the advice github gives for testing PRs is kinda wrong - if you just wanna test the branch as it stands without merging into master:

 git fetch git@github.com:silverwind/gitea fomantic-make-touch:fomantic-make-touch

[zeripath]

Thanks @silverwind this will save so much time - (and my fans coming on so much!)

[guillep2k]

the fomantic target for some reason does sometimes not update the timestamp on the destination directory, leading to useless rebuild.

Directory timestamps are not modified if no files are added/removed from them. So, your modification makes sense in case the step ran successfully over previously existing files.

However, regardless of this change, I'd like to point out that rule checking against directories is not a recommended practice:

Do not use directories as a dependency for generated targets, ever. Any activity within a directory will alter inodes forcing targets to always be stale.

If the Makefile step fails mid-way (npx eslint throws some error) and there has been a change in the directory, the rule will not trigger again.

Let me give you an example:

$(JS_DEST): $(JS_SOURCES) | node_modules
	npx eslint web_src/js webpack.config.js
	npx webpack --hide-modules --display-entrypoints=false
	@touch $(JS_DEST)

Let JS_DEST=public/js and JS_SOURCE=web_src/index.js, and let's ignore the node_modules dependency. To run this rule, make will check the timestamp of index.js against the timestamp of public/js. If index.js is newer, the rule will trigger and npx will be ran.

The problem comes if either npx eslint or npx webpack produce a change in public/js but fail and exit. Next time we run make, index.js will be older than public/js and the rule will not run, producing an invalid build.

An example of this situation is when you manually interrupt the build (CTRL+C), make some change for other reasons (e.g. edit some unrelated file, a .go source maybe) and run make again.

This will not affect our CI process, because CI steps either run or not, but are never interrupted and restarted. But we should be aware that the contents of public/js may be invalid after a failed or otherwise interrupted build. In those cases not even a make clean might be sufficient.

[guillep2k]

Adding to the problem above, we need to keep in mind that Makefile itself is somehow a dependency of the build process. When we switch branches through git checkout, we might get different versions of Makefile with different sets rules and that is not going to work well.

[silverwind]

Directory timestamps are not modified if no files are added/removed from them

Ah, I didn't know that, thanks. I think the manual touch is a good solution to that, thought it may probably be better to track individual files (which is a problem for output files as they need to be statically defined/kept up to date).

In those cases not even a make clean might be sufficient.

I'd expect users to run make clean-all from time to time in case of issues like above. Why do you think it would not help?

[guillep2k]

Ah, I didn't know that, thanks. I think the manual touch is a good solution to that,

Yes, touch {evidence} is the way to go. You need to be aware of all the files you need to generate, though, so those files are a dependency of {evidence}. It goes something like this:

evidence: file1 file2 file3
    some long and complicated command
    touch evidence

evidence itself must not be PHONY and should never be added to git.

I'd expect users to run make clean-all from time to time in case of issues like above. Why do you think it would not help?

Because we're having different versions of Makefile; for example:

• git checkout master
• make build <--- generates fomantic data
• git checkout release/v1.11
• make clean-all ---- this one won't know about fomantic.
• git status ---- will return some fomantic leftovers from the clean-all

I've found this problem in the past (I just clean manually).