doxygen/Makefile: do not rebuild riot.css automatically

[jcarrano]

Contribution description

The riot.css can be generated by the lessc tool from riot.less, butit is also tracked by git. If the Makefile detects that the user has the lessc tool, it automatically enables a rule to rebuild it.

This can be annoying to the user if he happens to have the tool in his machine and may also cause the docs built by the CI to differ to what is generated in user's PCs.

This commit removes the rule for generating riot.css and replaces it whith a phony target "riot-css" that must be explicitly called to recompile the stylesheet. Dependencies on that file are removed, since make no longer knows how to remake it.

Testing procedure

Install lesscpy from PIP and run make docs. The docs should be built fine.

If you touch doc/doxygen/src/css/riot.less nothing should get rebuild.

Typing make riot-css should unconditionally regenerate doc/doxygen/src/css/riot.css.

Issues/PRs references

I based it off #10237 (the travis commit is from that PR) but that is not strictly necessary.

Replaces #10239.

Fixes #8122

[jnohlgard]

A good improvement. See question inline regarding the implementation.

[jnohlgard]

This is somewhat ugly now with input files which are no longer dependencies of the target. How about only adding this to the original recipe specification?

.PHONY: riot-css
riot-css: src/css/riot.css

The removal of src/css/riot.css from the prerequisites of the html target should give the desired results anyway.

[jnohlgard]

you may want to list src/css/riot.css as .PHONY to get unconditional rebuild if that was desired.

[jcarrano]

src/css/variables.less should be prerequisite so that it gets updated from src/config.json each time riot-css is run.

Making src/css/riot.css a dependency of riot-css is the same as nothing, since the PHONY rule will be run anyways. The only difference is what happens when src/css/riot.css is missing (in one case lessc fails and in the other make fails before calling lessc).

[jnohlgard]

why is riot.less not a prerequisite anymore?

[jcarrano]

Because

1. riot-css is a PHONY target, so it will always get redone if the user types make riot-css
2. riot.less already exists and there is no way to "rebuild it".

Point (2) is the difference between riot.less and variables.less. The latter is automatically generated from a json, so make has to know that riot-css depends on it so that if the json changes, variables.less before remaking riot.css.

If I were to add riot.less to the prerequisites of riot-css it would not change anything because there is no rule to remake riot.less, make always considers it to be up to date.

I can do it anyways if it improves clarity.

[jnohlgard]

I can do it anyways if it improves clarity.

I would prefer it. There is no reason to remove correct dependencies even though riot.css is always rebuilt.

@cladmi what was the conclusion regarding .PHONY: deps vs recipe: FORCE? should riot.css depend on FORCE instead of being .PHONY?

[jcarrano]

If the name of the target was not riot-css but rather src/css/riot.css then it should be FORCE and not PHONY since it corresponds to a real file.

Here the target name is intentionally different from the file so that make never attempts to rebuild the file by itself. Then it should be PHONY.

Off topic: the most important conclusions of PHONY vs FORCE are that:

• For file-targets FORCE should be used instead of PHONY.
• If some target is being FORCEd, there is a high probability that the makefile has a design issue and that someone is sweeping problems under the rug.

[jcarrano]

This is getting quite annoying, having to checkout riot.css again each time I rebuild the docs. And if I forget and do a git commit -a then I have to unstage.

Can we merge this?

[cladmi]

I think we could keep only the src/css/riot.css target and just remove it from the dependencies of html. Having a phony target for rebuilding could help as shortcut but not sure if it is necessary.

I also noticed that re-creating the file with lesscpy discards the license header so may not even be suitable for generating it. It is documented in the lesscpy documentation:
https://lesscpy.readthedocs.io/en/latest/#differences-from-less-js
Re-creating the file with lessc gives me the same output as the tracked one.
I would remove the support for lesscpy completely in that case, or only support lesscpy and re-add the header manually. But a different PR to do it.

If the file can be stability re-created, a check in static-tests could be nice to check if it was changed but forgotten. I would see it as a different pull request though.
Or a lazyer solution would be that the target does a:

    @sed -i '1 i\/* This file was generated by "make $@" it should be manually updated when it dependencies are modified */' $@

[cladmi]

Summary, just removing src/css/riot.css from html dependencies could be enough for this PR.
The two .PHONY targets could also go in as they are only cleanup.

[cladmi]

One important thing to note on the testing procedure for master, you must have lesscpy installed but not lessc as with lessc the created file is the same as the versioned one.

[jcarrano]

I rebased this on master now that lessc is the only less supported.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.