Fix cfgman documentation release

[yadij]

... broken since release process moved to github releases.

[yadij]

Requires squid-cache/squid#2257 to have been merged to both v7 and v8 for this to work.

[rousskov]

AFAICT, this particular command sequence ignores failures and lets the current iteration proceed to the "update DYN documents" step that, AFAICT, depends on the failed command. If we do not want to abort the script on failures, should not we proceed to the next iteration instead?

[yadij]

This command sequence need to either entirely succeed, or none of the changes are usable.

It will either reach the mv which performs "publish successful changes" action. Or, it will drop out to the cleanup step following.

Logic:

• try to build+publish
• on success: cleanup
• on error: cleanup + exit

[rousskov]

A checkout command does not belong to a "clean workspace" function. The command itself looks a bit odd.

If you want to erase changes to tracked files, then use something like git reset --hard. If tracked files may remain as is, then remove this command (from this function). Let the caller checkout what they need (one caller already does, resulting in two sequential checkouts).

[yadij]

The checkout -- . replaces any modified files with their repository committed versions. A reset only changes what a repository considers its "committed version" to be.

$ git diff configure.ac
$ echo "#FOO" >> configure.ac  && git reset -- configure.ac
Unstaged changes after reset:
M	configure.ac
$ git diff configure.ac
diff --git a/configure.ac b/configure.ac
index 8fa781e4db..2ddaba4ffa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2598,3 +2598,4 @@ AC_CONFIG_FILES([
 #AC_CONFIG_SUBDIRS(libltdl)
 
 AC_OUTPUT
+#FOO

$ git diff configure.ac
echo "#FOO" >> configure.ac  && git reset --hard -- configure.ac
fatal: Cannot do hard reset with paths.
$ git diff configure.ac
diff --git a/configure.ac b/configure.ac
index 8fa781e4db..2ddaba4ffa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2598,3 +2598,4 @@ AC_CONFIG_FILES([
 #AC_CONFIG_SUBDIRS(libltdl)
 
 AC_OUTPUT
+#FOO

$ git diff configure.ac
$ echo "#FOO" >> configure.ac  && git checkout -- configure.ac
$ git diff configure.ac

[rousskov]

It is strange to check whether this directory is present when the above commands rely on its presence to store their result. Should not this test be moved higher, where it used to reside before this PR?

[yadij]

This is the error detection and handling you missed on the command sequence at line 21-23.
It exists here to avoid duplicating calls to the cleanup.

FYI, it looks a bit odd because the cfgman published area may also have content from a previous publication. In which case this logic is run to ensure that is still correct.

[rousskov]

Does this update use some make-generated files? If yes, we should not clean the working directory before this update (and we should not update if make fails). If not, I recommend rephrasing this comment to clarify where the new information is coming from, to remove the current implication that this information is the result of the above make.

[yadij]

All following logic in this script uses the result (if any) of the above mv publication command within the $SQUID_WWW_PATH area. If any above logic failed, then we likely have garbage in the checkout area that needs to be purged.