Makefile/repos: new formatting options, make commit, make checkout

[simon-skylabs]

This PR defines new repo commands in the Makefile rules:

• make COMMIT_MESSAGE="foo" commit
• make SELECT_BRANCH=branch/name checkout
  Checks out branch/name in each repo and creates the branch if it does not exist.

Option for make gitclean: by default, it works as dry run, the effect can be applied with make GITCLEAN_DRY_RUN=no gitclean

Formatting: by default, the output of git commands is double spaced and indented. The formatting can be changed with:

• make 'GIT_FORMATTING=--indent --skip-first-line' git-command: double spaced and indented;
• make 'GIT_FORMATTING=--skip-first-line' git-command: only double spaced;
• make 'GIT_FORMATTING=--indent' git-command: only indented;
• make 'GIT_FORMATTING=' git-command: unformatted.

[rlepigre-skylabs-ai]

I like most of the changes (up to details), but I'm not a big fan of the formatting script. The code is already pretty complicated as it stands, and I'm not sure adding another indirection is reasonable.

It seems like the script means to configure mostly two things: whether some space is inserted before the output for a particular repo, and whether the git output is indented. Couldn't we easily control both from the Makefile, without going through a script?

Another potential issue is that going through sed (you're using gsed at the moment and that won't work for everyone) will disable Git output colours I believe. I guess we could define a GITINDENT variable in the Makefile, whose value could be something like -c color.ui=always | sed 's/^/ /', and then replace git ... with git ... $GITINDENT to keep the colors.

[cipher1024]

Another potential issue is that going through sed (you're using gsed at the moment and that won't work for everyone) will disable Git output colours I believe. I guess we could define a GITINDENT variable in the Makefile, whose value could be something like -c color.ui=always | sed 's/^/ /', and then replace git ... with git ... $GITINDENT to keep the colors.

I hadn't thought of the colors. That sounds like it could work. For git ... $GITINDENT though, we'd need the pipe to be parsed like a pipe and not part of one last list of arguments. Is this how make variable expansion works?

Couldn't we easily control both from the Makefile, without going through a script?
I'm not sure adding another indirection is reasonable.

I started off by writing it as conditionals in the Makefile but it seems to give too much space to formatting in my view. Do you think that can be done more discreetly? In particular, how do you think you would do the double spacing?

[rlepigre-skylabs-ai]

Another potential issue is that going through sed (you're using gsed at the moment and that won't work for everyone) will disable Git output colours I believe. I guess we could define a GITINDENT variable in the Makefile, whose value could be something like -c color.ui=always | sed 's/^/ /', and then replace git ... with git ... $GITINDENT to keep the colors.

I hadn't thought of the colors. That sounds like it could work. For git ... $GITINDENT though, we'd need the pipe to be parsed like a pipe and not part of one last list of arguments. Is this how make variable expansion works?

My suggestion actually does not work, since the -c color.ui=always bit needs to be before the -C...

Couldn't we easily control both from the Makefile, without going through a script?
I'm not sure adding another indirection is reasonable.

I started off by writing it as conditionals in the Makefile but it seems to give too much space to formatting in my view. Do you think that can be done more discreetly? In particular, how do you think you would do the double spacing?

I think we might be able to get something satisfactory. The best I was able to do so far is the following, which only ports the peek targets, but other targets could be easily adapted:

diff --git a/dev/repos/rules.mk b/dev/repos/rules.mk
index 1b7bd97..b66c136 100644
--- a/dev/repos/rules.mk
+++ b/dev/repos/rules.mk
@@ -1,5 +1,30 @@
 include dev/repos/config.mk

+WORKSPACE_GIT_LEADING_BLANK_LINES ?= 0
+WORKSPACE_GIT_INDENT ?= 0
+
+ifeq (${WORKSPACE_GIT_LEADING_BLANK_LINES},0)
+define header
+       @echo $1
+endef
+else
+define header
+       @printf '\n%.0s' {1..${WORKSPACE_GIT_LEADING_BLANK_LINES}}
+       @echo $1
+endef
+endif
+
+ifeq (${WORKSPACE_GIT_INDENT},0)
+define git
+       $(Q)git -C $1 $2 $3
+endef
+else
+GIT_INDENT_PREFIX = $(shell printf ' %.0s' {1..${WORKSPACE_GIT_INDENT}})
+define git
+       $(Q)git -c color.ui=always -C $1 $2 $3 | sed 's/^/${GIT_INDENT_PREFIX}/'
+endef
+endif
+
 REPO_GROUPS = upstream owned downstream public private

 define subrepo_targets
@@ -140,9 +165,8 @@ ${REPO_MODE}_PEEK_TARGETS += peek-${REPO_NAME}
 .PHONY: peek-${REPO_NAME}
 peek-${REPO_NAME}:
 ifeq ($(wildcard ${REPO_DIR}),${REPO_DIR})
-       @echo ""
-       @echo "Peeking into ${REPO_DIR}:"
-       @git -C ${REPO_DIR} status --short --branch --untracked-files=normal ${GIT_PEEK_OPTS}
+       $(call header,"Peeking into ${REPO_DIR}")
+       $(call git,${REPO_DIR},status,--short --branch --untracked-files=normal ${GIT_PEEK_OPTS})
 else
        @echo "No repository in ${REPO_DIR}, cannot peek."
 endif
@@ -328,9 +352,8 @@ push: push-workspace ${PUSH_TARGETS}

 .PHONY: peek-workspace
 peek-workspace:
-       @echo ""
-       @echo "Peeking into ./"
-       @git status --short --branch --untracked-files=normal ${GIT_PEEK_OPTS}
+       $(call header,"Peeking into ./")
+       $(call git,./,status,--short --branch --untracked-files=normal ${GIT_PEEK_OPTS})

 .PHONY: peek
 peek: peek-workspace ${PEEK_TARGETS}

I can't say I really love that code, but maybe it's OK. What do you think?

[pgiarrusso-sl]

Since I like @rlepigre-skylabs-ai 's feedback, I'll let him drive this work. Unassigned. But ping me if there's a need!

[simon-skylabs]

I can't say I really love that code, but maybe it's OK. What do you think?

I considered going in that direction, internally redefining git or also putting something in the Q prefix. What made me choose to write it in a bash file was to avoid filling up the makefiles with that code. All this being said, your version is shorter and simpler. We may be able to get it out of the way if we want by including a separate .mk file. I'm not super fluent with Makefile so I can't say what we'll find easier to work with in the future. I think you can make the best call in that matter.