From 168ebb7159584df425797dd13d8d8986dc08051e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 27 Nov 2024 22:23:45 +0900 Subject: CodingGuidelines: a handful of error message guidelines It is more efficient to have something in the coding guidelines document to point at, when we want to review and comment on a new message in the codebase to make sure it "fits" in the set of existing messages. Let's write down established best practice we are aware of. Helped-by: Eric Sunshine Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) (limited to 'Documentation/CodingGuidelines') diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 3263245b03..31714227ca 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -689,16 +689,30 @@ Program Output Error Messages - - Do not end error messages with a full stop. + - Do not end a single-sentence error message with a full stop. - Do not capitalize the first word, only because it is the first word - in the message ("unable to open %s", not "Unable to open %s"). But + in the message ("unable to open '%s'", not "Unable to open '%s'"). But "SHA-3 not supported" is fine, because the reason the first word is capitalized is not because it is at the beginning of the sentence, but because the word would be spelled in capital letters even when it appeared in the middle of the sentence. - - Say what the error is first ("cannot open %s", not "%s: cannot open") + - Say what the error is first ("cannot open '%s'", not "%s: cannot open"). + + - Enclose the subject of an error inside a pair of single quotes, + e.g. `die(_("unable to open '%s'"), path)`. + + - Unless there is a compelling reason not to, error messages from + porcelain commands should be marked for translation, e.g. + `die(_("bad revision %s"), revision)`. + + - Error messages from the plumbing commands are sometimes meant for + machine consumption and should not be marked for translation, + e.g., `die("bad revision %s", revision)`. + + - BUG("message") are for communicating the specific error to developers, + thus should not be translated. Externally Visible Names -- cgit v1.3 From 95bcd6f0b709ec6e761270732946651757cc11c3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 6 Dec 2024 14:24:50 +0100 Subject: Makefile: allow "bin-wrappers/" directory to exist The "bin-wrappers/" directory gets created by our build system and is populated with one script for each of our binaries. There isn't anything inherently wrong with the current layout, but it is somewhat hard to adapt for out-of-tree build systems. Adapt the layout such that our "bin-wrappers/" directory always exists and contains our "wrap-for-bin.sh" script to make things a little bit easier for subsequent steps. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- .gitignore | 1 - Documentation/CodingGuidelines | 2 +- Makefile | 6 +++--- bin-wrappers/.gitignore | 9 +++++++++ bin-wrappers/wrap-for-bin.sh | 36 ++++++++++++++++++++++++++++++++++++ contrib/buildsystems/CMakeLists.txt | 6 +++--- wrap-for-bin.sh | 36 ------------------------------------ 7 files changed, 52 insertions(+), 44 deletions(-) create mode 100644 bin-wrappers/.gitignore create mode 100755 bin-wrappers/wrap-for-bin.sh delete mode 100644 wrap-for-bin.sh (limited to 'Documentation/CodingGuidelines') diff --git a/.gitignore b/.gitignore index d3be460040..e82aa19df0 100644 --- a/.gitignore +++ b/.gitignore @@ -12,7 +12,6 @@ /GIT-TEST-SUITES /GIT-USER-AGENT /GIT-VERSION-FILE -/bin-wrappers/ /git /git-add /git-am diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 87904791cb..1df9d0c42f 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -583,7 +583,7 @@ For C programs: Run `GIT_DEBUGGER=1 ./bin-wrappers/git foo` to simply use gdb as is, or run `GIT_DEBUGGER=" " ./bin-wrappers/git foo` to use your own debugger and arguments. Example: `GIT_DEBUGGER="ddd --gdb" - ./bin-wrappers/git log` (See `wrap-for-bin.sh`.) + ./bin-wrappers/git log` (See `bin-wrappers/wrap-for-bin.sh`.) - The primary data structure that a subsystem 'S' deals with is called `struct S`. Functions that operate on `struct S` are named diff --git a/Makefile b/Makefile index 009e7d8e95..6e93319bed 100644 --- a/Makefile +++ b/Makefile @@ -3202,8 +3202,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $( all:: $(TEST_PROGRAMS) $(test_bindir_programs) $(UNIT_TEST_PROGS) $(CLAR_TEST_PROG) -bin-wrappers/%: wrap-for-bin.sh - $(call mkdir_p_parent_template) +$(test_bindir_programs): bin-wrappers/%: bin-wrappers/wrap-for-bin.sh $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ -e 's|@BUILD_DIR@|$(shell pwd)|' \ -e 's|@PROG@|$(patsubst test-%,t/helper/test-%,$(@F))$(if $(filter-out $(BINDIR_PROGRAMS_NO_X),$(@F)),$(X),)|' < $< > $@ && \ @@ -3700,7 +3699,8 @@ clean: profile-clean coverage-clean cocciclean $(RM) $(SP_OBJ) $(RM) $(HCC) $(RM) version-def.h - $(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json + $(RM) -r $(dep_dirs) $(compdb_dir) compile_commands.json + $(RM) $(test_bindir_programs) $(RM) -r po/build/ $(RM) *.pyc *.pyo */*.pyc */*.pyo $(GENERATED_H) $(ETAGS_TARGET) tags cscope* $(RM) -r .dist-tmp-dir .doc-tmp-dir diff --git a/bin-wrappers/.gitignore b/bin-wrappers/.gitignore new file mode 100644 index 0000000000..1c6c90458b --- /dev/null +++ b/bin-wrappers/.gitignore @@ -0,0 +1,9 @@ +/git +/git-cvsserver +/git-receive-pack +/git-shell +/git-upload-archive +/git-upload-pack +/scalar +/test-fake-ssh +/test-tool diff --git a/bin-wrappers/wrap-for-bin.sh b/bin-wrappers/wrap-for-bin.sh new file mode 100755 index 0000000000..7898a1c238 --- /dev/null +++ b/bin-wrappers/wrap-for-bin.sh @@ -0,0 +1,36 @@ +#!/bin/sh + +# wrap-for-bin.sh: Template for git executable wrapper scripts +# to run test suite against sandbox, but with only bindir-installed +# executables in PATH. The Makefile copies this into various +# files in bin-wrappers, substituting +# @BUILD_DIR@ and @PROG@. + +GIT_EXEC_PATH='@BUILD_DIR@' +if test -n "$NO_SET_GIT_TEMPLATE_DIR" +then + unset GIT_TEMPLATE_DIR +else + GIT_TEMPLATE_DIR='@BUILD_DIR@/templates/blt' + export GIT_TEMPLATE_DIR +fi +GITPERLLIB='@BUILD_DIR@/perl/build/lib'"${GITPERLLIB:+:$GITPERLLIB}" +GIT_TEXTDOMAINDIR='@BUILD_DIR@/po/build/locale' +PATH='@BUILD_DIR@/bin-wrappers:'"$PATH" + +export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR + +case "$GIT_DEBUGGER" in +'') + exec "${GIT_EXEC_PATH}/@PROG@" "$@" + ;; +1) + unset GIT_DEBUGGER + exec gdb --args "${GIT_EXEC_PATH}/@PROG@" "$@" + ;; +*) + GIT_DEBUGGER_ARGS="$GIT_DEBUGGER" + unset GIT_DEBUGGER + exec ${GIT_DEBUGGER_ARGS} "${GIT_EXEC_PATH}/@PROG@" "$@" + ;; +esac diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index e7bd45cbb6..9348665f22 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1095,20 +1095,20 @@ set(wrapper_test_scripts foreach(script ${wrapper_scripts}) - file(STRINGS ${CMAKE_SOURCE_DIR}/wrap-for-bin.sh content NEWLINE_CONSUME) + file(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME) string(REPLACE "@BUILD_DIR@" "${CMAKE_BINARY_DIR}" content "${content}") string(REPLACE "@PROG@" "${script}${EXE_EXTENSION}" content "${content}") file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content}) endforeach() foreach(script ${wrapper_test_scripts}) - file(STRINGS ${CMAKE_SOURCE_DIR}/wrap-for-bin.sh content NEWLINE_CONSUME) + file(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME) string(REPLACE "@BUILD_DIR@" "${CMAKE_BINARY_DIR}" content "${content}") string(REPLACE "@PROG@" "t/helper/${script}${EXE_EXTENSION}" content "${content}") file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/${script} ${content}) endforeach() -file(STRINGS ${CMAKE_SOURCE_DIR}/wrap-for-bin.sh content NEWLINE_CONSUME) +file(STRINGS ${CMAKE_SOURCE_DIR}/bin-wrappers/wrap-for-bin.sh content NEWLINE_CONSUME) string(REPLACE "@BUILD_DIR@" "${CMAKE_BINARY_DIR}" content "${content}") string(REPLACE "@PROG@" "git-cvsserver" content "${content}") file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/git-cvsserver ${content}) diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh deleted file mode 100644 index 7898a1c238..0000000000 --- a/wrap-for-bin.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/bin/sh - -# wrap-for-bin.sh: Template for git executable wrapper scripts -# to run test suite against sandbox, but with only bindir-installed -# executables in PATH. The Makefile copies this into various -# files in bin-wrappers, substituting -# @BUILD_DIR@ and @PROG@. - -GIT_EXEC_PATH='@BUILD_DIR@' -if test -n "$NO_SET_GIT_TEMPLATE_DIR" -then - unset GIT_TEMPLATE_DIR -else - GIT_TEMPLATE_DIR='@BUILD_DIR@/templates/blt' - export GIT_TEMPLATE_DIR -fi -GITPERLLIB='@BUILD_DIR@/perl/build/lib'"${GITPERLLIB:+:$GITPERLLIB}" -GIT_TEXTDOMAINDIR='@BUILD_DIR@/po/build/locale' -PATH='@BUILD_DIR@/bin-wrappers:'"$PATH" - -export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR - -case "$GIT_DEBUGGER" in -'') - exec "${GIT_EXEC_PATH}/@PROG@" "$@" - ;; -1) - unset GIT_DEBUGGER - exec gdb --args "${GIT_EXEC_PATH}/@PROG@" "$@" - ;; -*) - GIT_DEBUGGER_ARGS="$GIT_DEBUGGER" - unset GIT_DEBUGGER - exec ${GIT_DEBUGGER_ARGS} "${GIT_EXEC_PATH}/@PROG@" "$@" - ;; -esac -- cgit v1.3