Skip to content

Commit

Permalink
advice-type.h: add a generated list of advice, like hook-list.h
Browse files Browse the repository at this point in the history
Make the addition of new advice to advice.[ch] simpler and more
fool-proof by generating the list of advice enums from
Documentation/config/advice.txt, instead of requiring new additions to
be hardcoded across the three files (*.c, *.h and *.txt).

In subsequent commits we'll also start auto-generating the mapping
between enum labels and config keys (in advice.c). I.e. we'll only
define these in advice.txt, and not in advice.c. But for now let's get
rid of 1/3 of the duplicate places we declare these.

This change is only practically possible due to the preceding fixes to
"COMPUTE_HEADER_DEPENDENCIES" being extended here with
"dep_template". I.e. we had generated headers before this, but for any
file that relied on them we needed to manually declare the dependency
on the generated header, see my d3fd1a6 (Makefile: correct the
dependency graph of hook-list.h, 2021-12-17) for a case where I got
that wrong.

Unlike this new advice-type.h those cases are easy, each of our
current generated headers is used in exactly one place, but this one
needs to be added to advice.h, which is in turn included in
cache.h.

Even if we had the code that needs advice.h manually include it (I
tried, and ejecting it from cache.h would probably be worthwhile)
maintaining the dependency mapping would be fragile, although we could
brute-force check its validity in CI by compiling every *.o file in a
"make clean && make <file>.o" loop.

But as this change shows we can do better. We have the *.o and *.s
files depend on "$(COMMON_GENERATED_H)" under
COMPUTE_HEADER_DEPENDENCIES , but only if we haven't compiled it once
before. We can thus benefit from the accurate dependencies the
compiler generated. This means that e.g. this works:

    $ git clean -qdxf; make grep.o; git clean -dxf '*.h' '*.o'; make grep.o
    GIT_VERSION = 2.35.1.475.g534253553cd.dirty
        * new build flags
        MKDIR -p .build/dep
        GEN advice-type.h
        CC grep.o
    Removing advice-type.h
    Removing grep.o
        CC grep.o

I.e. note how we have grep.o over-depend on advice-type.h, but only
the first time around. The second time around we looked at the
dependency graph in ".build/dep/grep.o.d". Doing the same with
e.g. remote.o (which does use the generated header) will end in:

    Removing advice-type.h
    Removing remote.o
        GEN advice-type.h
        CC remote.o

I.e. in that case we do still depend on advice-type.h post-compilation:

    $ grep  -c advice-type.h .build/dep/remote.o.d
    2
    $

We would need one additional special-case: The advice-type.h header is
the only generated header that's included in another header, so for
"make hdr-check" we need to have advice.hco depend on
advice-type.h. Let's instead just have all *.hco depend on these
$(COMMON_GENERATED_H), it will make this less fragile going forward.

See cfe853e (hook-list.h: add a generated list of hooks, like
config-list.h, 2021-09-26) for similar prior art, including a similar
addition to CMakeLists.txt.

generate-advice.sh portability notes: the $HT is needed due to
e.g. AIX's "sed". Solaris's "sed" and "tr" then don't understand
the (non-portable) "[A-Z]" or "A-Z" syntax, so POSIX character classes
are being used here.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
  • Loading branch information
avar committed Oct 22, 2022
1 parent 48af13a commit a00f1cb
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 46 deletions.
1 change: 1 addition & 0 deletions .gitignore
Expand Up @@ -190,6 +190,7 @@
/gitweb/gitweb.cgi
/gitweb/static/gitweb.js
/gitweb/static/gitweb.min.*
/advice-type.h
/config-list.h
/command-list.h
/hook-list.h
Expand Down
51 changes: 51 additions & 0 deletions Makefile
Expand Up @@ -667,9 +667,11 @@ PTHREAD_LIBS = -lpthread
# Guard against environment variables
BUILTIN_OBJS =
BUILT_INS =
COMMON_GENERATED_H =
COMPAT_CFLAGS =
COMPAT_OBJS =
XDIFF_OBJS =
GENERATED_ADVICE_H =
GENERATED_H =
EXTRA_CPPFLAGS =
FUZZ_OBJS =
Expand Down Expand Up @@ -927,6 +929,14 @@ XDIFF_LIB = xdiff/lib.a
REFTABLE_LIB = reftable/libreftable.a
REFTABLE_TEST_LIB = reftable/libreftable_test.a

# Generated headers. The "COMMON_GENERATED_H" will be added as
# implicit dependencies on fresh builds (without existing dep/*.mak
# files)
GENERATED_ADVICE_H += advice-type.h

COMMON_GENERATED_H += $(GENERATED_ADVICE_H)

GENERATED_H += $(COMMON_GENERATED_H)
GENERATED_H += command-list.h
GENERATED_H += config-list.h
GENERATED_H += hook-list.h
Expand Down Expand Up @@ -2366,6 +2376,9 @@ $(BUILT_INS): git$X
ln -s $< $@ 2>/dev/null || \
cp $< $@

$(GENERATED_ADVICE_H): generate-advice.sh Documentation/config/advice.txt
$(QUIET_GEN)$(SHELL_PATH) ./generate-advice.sh $@ >$@

config-list.h: generate-configlist.sh

config-list.h: Documentation/*config.txt Documentation/config/*.txt
Expand Down Expand Up @@ -2644,6 +2657,43 @@ dep_file = .build/dep/$(basename $@).mak
dep_args = -MF $(dep_file) $(foreach s,$(dep_exts),-MQ $(basename $@)$(s)) -MMD -MP
asm_deps =
asm_dep_args = -c $(dep_args)

# Generate different dependencies for objects that have corresponding
# dep/*.mak files, and those that don't.
OBJECTS_DEP_PRESENT = $(wildcard $(OBJECTS_DEP))
OBJECTS_DEP_PRESENT_BASENAME = $(OBJECTS_DEP_PRESENT:.build/dep/%.mak=%)
OBJECTS_DEP_MISSING_BASENAME = $(filter-out $(OBJECTS_DEP_PRESENT_BASENAME),$(OBJECTS_BASENAME))

# The generated $$(COMMON_GENERATED_H) are widespread enough that
# exhaustively listing their dependencies would be painful, so let's
# fall back on having all *.$(dep_exts) depend on them IFF there is no
# corresponding dep/*.mak file listing the actual dependencies.
define dep-line

$(1): $(2)
endef
define dep-lines
$(foreach e,$(dep_exits),$(call dep-line,$(1)$(e),$(2)))
endef

define dep-missing
$(call dep-lines,$(1),$$(COMMON_GENERATED_H))
endef

define dep-present
# Dependencies for $(1) provided by generated *.mak

endef

# The "dep-present" isn't strictly needed (it only makes comments),
# but makes ad-hoc debugging & inspection easier.
define computed-deps
$(foreach f,$(OBJECTS_DEP_PRESENT_BASENAME),$(call dep-present,$(f)))
$(foreach f,$(OBJECTS_DEP_MISSING_BASENAME),$(call dep-missing,$(f)))
endef

$(eval $(call computed-deps))

else
$(OBJECTS): $(LIB_H) $(GENERATED_H)
endif
Expand Down Expand Up @@ -3187,6 +3237,7 @@ HCC = $(HCO:hco=hcc)
@echo '#include "git-compat-util.h"' >$@
@echo '#include "$<"' >>$@

$(HCO): $(COMMON_GENERATED_H)
$(HCO): %.hco: %.hcc FORCE
$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<

Expand Down
47 changes: 1 addition & 46 deletions advice.h
Expand Up @@ -2,55 +2,10 @@
#define ADVICE_H

#include "git-compat-util.h"
#include "advice-type.h"

struct string_list;

/*
* To add a new advice, you need to:
* Define a new advice_type.
* Add a new entry to advice_setting array.
* Add the new config variable to Documentation/config/advice.txt.
* Call advise_if_enabled to print your advice.
*/
enum advice_type {
ADVICE_ADD_EMBEDDED_REPO,
ADVICE_ADD_EMPTY_PATHSPEC,
ADVICE_ADD_IGNORED_FILE,
ADVICE_AM_WORK_DIR,
ADVICE_AMBIGUOUS_FETCH_REFSPEC,
ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
ADVICE_COMMIT_BEFORE_MERGE,
ADVICE_DETACHED_HEAD,
ADVICE_SUGGEST_DETACHING_HEAD,
ADVICE_FETCH_SHOW_FORCED_UPDATES,
ADVICE_GRAFT_FILE_DEPRECATED,
ADVICE_IGNORED_HOOK,
ADVICE_IMPLICIT_IDENTITY,
ADVICE_NESTED_TAG,
ADVICE_OBJECT_NAME_WARNING,
ADVICE_PUSH_ALREADY_EXISTS,
ADVICE_PUSH_FETCH_FIRST,
ADVICE_PUSH_NEEDS_FORCE,
ADVICE_PUSH_NON_FF_CURRENT,
ADVICE_PUSH_NON_FF_MATCHING,
ADVICE_PUSH_UNQUALIFIED_REFNAME,
ADVICE_PUSH_UPDATE_REJECTED,
ADVICE_PUSH_REF_NEEDS_UPDATE,
ADVICE_RESET_NO_REFRESH,
ADVICE_RESOLVE_CONFLICT,
ADVICE_RM_HINTS,
ADVICE_SEQUENCER_IN_USE,
ADVICE_SET_UPSTREAM_FAILURE,
ADVICE_STATUS_AHEAD_BEHIND,
ADVICE_STATUS_HINTS,
ADVICE_STATUS_UOPTION,
ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
ADVICE_SUBMODULES_NOT_UPDATED,
ADVICE_UPDATE_SPARSE_PATH,
ADVICE_WAITING_FOR_EDITOR,
ADVICE_SKIPPED_CHERRY_PICKS,
};

int git_default_advice_config(const char *var, const char *value);
__attribute__((format (printf, 1, 2)))
void advise(const char *advice, ...);
Expand Down
7 changes: 7 additions & 0 deletions contrib/buildsystems/CMakeLists.txt
Expand Up @@ -671,6 +671,13 @@ if(NOT EXISTS ${CMAKE_BINARY_DIR}/hook-list.h)
OUTPUT_FILE ${CMAKE_BINARY_DIR}/hook-list.h)
endif()

if(NOT EXISTS ${CMAKE_BINARY_DIR}/advice-type.h)
message("Generating advice-type.h")
execute_process(COMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/generate-advice.sh advice-type.h
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
OUTPUT_FILE ${CMAKE_BINARY_DIR}/advice-type.h)
endif()

include_directories(${CMAKE_BINARY_DIR})

#build
Expand Down
67 changes: 67 additions & 0 deletions generate-advice.sh
@@ -0,0 +1,67 @@
#!/bin/sh

HT=' '

advice_list () {
sed -n \
-e '/^advice.*::$/d' \
-e "/::/ {
s/^$HT//;
s/::\$//;
p;
}" \
<Documentation/config/advice.txt
}

txt2label () {
sed \
-e 's/\([^_]\)\([[:upper:]]\)/\1_\2/g' \
-e 's/^/ADVICE_/' |
tr '[:lower:]' '[:upper:]'
}

advice_labels () {
advice_list |
txt2label
}

listify () {
sed \
-e "2,\$s/^/$HT/" \
-e 's/$/,/'
}

advice_labels_manual () {
cat <<-\EOF
ADVICE_GRAFT_FILE_DEPRECATED
ADVICE_OBJECT_NAME_WARNING
ADVICE_SET_UPSTREAM_FAILURE
EOF
}

case "$#" in
1) ;;
*)
echo "usage: $0 advice-type.h >advice-type.h"
exit 1
esac

case "$1" in
advice-type.h)
cat <<EOF
/* Automatically generated by generate-advice.sh */
enum advice_type {
/* Manually listed, needs adding to Documentation/config/advice.txt */
$(advice_labels_manual | listify)
/* Auto-generated from Documentation/config/advice.txt */
$(advice_labels | listify)
};
EOF
;;
*)
echo "$0: unknown target $1" >&2
exit 1
;;
esac

0 comments on commit a00f1cb

Please sign in to comment.