[PATCH v2] t4253-am-keep-cr-dos: avoid using pipes
The exit code of the upstream in a pipe is ignored thus we should avoid using it. By writing out the output of the git command to a file, we can test the exit codes of both the commands. Signed-off-by: Boxuan Li --- Thanks to Eric Sunshine's review, I've removed spaces after '>' and changed 'actual' into 'output'. --- t/t4253-am-keep-cr-dos.sh | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t4253-am-keep-cr-dos.sh b/t/t4253-am-keep-cr-dos.sh index 553fe3e88e..6e1b73ec3a 100755 --- a/t/t4253-am-keep-cr-dos.sh +++ b/t/t4253-am-keep-cr-dos.sh @@ -51,14 +51,16 @@ test_expect_success 'am with dos files without --keep-cr' ' test_expect_success 'am with dos files with --keep-cr' ' git checkout -b dosfiles-keep-cr initial && - git format-patch -k --stdout initial..master | git am --keep-cr -k -3 && + git format-patch -k --stdout initial..master >output && + git am --keep-cr -k -3 output && git diff --exit-code master ' test_expect_success 'am with dos files config am.keepcr' ' git config am.keepcr 1 && git checkout -b dosfiles-conf-keepcr initial && - git format-patch -k --stdout initial..master | git am -k -3 && + git format-patch -k --stdout initial..master >output && + git am -k -3 output && git diff --exit-code master ' -- 2.21.0.777.g83232e3864
Re: Git config "ignorecase = true" has issues
Thanks for replying. Do you know how the query you mentioned ("Do we have a file named "file.txt" on disk?") is implemented in git? I'd suggest to use the C++ Windows API function FindFirstFileW() and then manually compare the result with the queried file name, like: bool fileExists(const WCHAR_T *fileName, bool ignoreCase) { WIN32_FIND_DATAW fileData; bool fileExists = false; int (*strCmp)(const wchar_t *string1, const wchar_t *string2, size_t count) = ignoreCase ? _wcsnicmp : wcsncmp; ); if (FindFirstFileW(fileName, fileData) != INVALID_HANDLE_VALUE) { if (!strCmp(fileName, fileData.cFileName, wcsnlen(fileName, MAX_PATH) + 1)) fileExists = true; FindClose(fileData); } return fileExists; } --- Gesendet: Mittwoch, 24. April 2019 um 21:11 Uhr Von: "Torsten Bögershausen" An: "Ax Da" Cc: git@vger.kernel.org Betreff: Re: Git config "ignorecase = true" has issues On Fri, Apr 19, 2019 at 09:28:32PM +0200, Ax Da wrote: > > We're working on Windows machines and have been experiencing issues with the > current implementation of Git with config setting "core.ignorecase = true" > (which is the default on Windows machines and repositories created on Windows > machines): > > Renaming files in a repository by only changing their case (changing a > capital letter to its small equivalent and vice versa) is ignored by Git. Git > retains the original case in the repository and all contributors will > continue to see the ole file name which leads to confusion and issues with > Open Source tools programmed to not ignore file name case. > > Currently there is no way to convey the new file name (only differing in > case) to Git when "core.ignorecase = true". > > Hence, I propose to alter the behaviour of Git when "core.ignorecase = true": > A repository's file name changes should be recognized as a RENAME operation > and be propagated to the repository even when the new file name only differs > from the old file name in case. You can rename files like this: git mv File.txt file.txt git commit and Git will record the changes. The main problem is, that after the rename, and may be on another machine after a pull, Git checks with the file system, if any updates in the working tree are needed. In human speech, Git asks the file system: Do we have a file named "file.txt" on disk ? And Windows answers: Yes we have. Even if the file is named "File.txt" and Git asks for "file.txt". You can try it yourself. Run cat File.txt under the Git shell (bash) or type File.txt under cmd.exe That is how it is. If you really need the updated name "file.txt", you can delete all files in the worktree rm -rf * followed by git reset --hard But in any case, run git status before and make sure that your working tree is clean. > > Thanks, > Axel Dahmen
git has issues with international characters in branch names
The documentation doesn't seem to be clear about which characters are allowed as branch names. We have been using umlauts and apostrophes in our branch names, and both are having issues: - apostrophe: git removes the apostrophes from the branch name when creating/pushing a branch. In our TFS server repository we're having the same branch name twice: Once with apostrophes, and once without. And I don't seem to be able to delete the one without apostrophes. - umlauts: The umlauts are not correctly interpreted when SMB is used. "gemäss" becomes "gemss". (See images attached.)
[PATCH v3 2/4] revisions.txt: mark optional rev arguments with []
In revisions.txt, an optional rev argument was not distinguised. Instead, a user had to continue and read the description in order to learn that the argument was optional. Since the [] notation for an optional argument is common-knowledge in the Git documentation, mark optional arguments with [] so that it's more obvious for the reader. Helped-by: Andreas Heiduk Signed-off-by: Denton Liu --- Documentation/revisions.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index e5f11691b1..dd99770c47 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -65,7 +65,7 @@ some output processing may assume ref names in UTF-8. '@':: '@' alone is a shortcut for `HEAD`. -'@{}', e.g. 'master@\{yesterday\}', 'HEAD@{5 minutes ago}':: +'[]@{}', e.g. 'master@\{yesterday\}', 'HEAD@{5 minutes ago}':: A ref followed by the suffix '@' with a date specification enclosed in a brace pair (e.g. '\{yesterday\}', '{1 month 2 weeks 3 days 1 hour 1 @@ -95,7 +95,7 @@ some output processing may assume ref names in UTF-8. The construct '@{-}' means the th branch/commit checked out before the current one. -'@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}':: +'[]@\{upstream\}', e.g. 'master@\{upstream\}', '@\{u\}':: The suffix '@\{upstream\}' to a branchname (short form '@\{u\}') refers to the branch that the branch specified by branchname is set to build on top of (configured with `branch..remote` and @@ -103,7 +103,7 @@ some output processing may assume ref names in UTF-8. current one. These suffixes are also accepted when spelled in uppercase, and they mean the same thing no matter the case. -'@\{push\}', e.g. 'master@\{push\}', '@\{push\}':: +'[]@\{push\}', e.g. 'master@\{push\}', '@\{push\}':: The suffix '@\{push}' reports the branch "where we would push to" if `git push` were run while `branchname` was checked out (or the current `HEAD` if no branchname is specified). Since our push destination is @@ -131,7 +131,7 @@ from one location and push to another. In a non-triangular workflow, This suffix is also accepted when spelled in uppercase, and means the same thing no matter the case. -'{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0':: +'{caret}[]', e.g. 'HEAD{caret}, v1.5.1{caret}0':: A suffix '{caret}' to a revision parameter means the first parent of that commit object. '{caret}' means the th parent (i.e. '{caret}' @@ -302,7 +302,7 @@ The 'r1{caret}@' notation means all parents of 'r1'. The 'r1{caret}!' notation includes commit 'r1' but excludes all of its parents. By itself, this notation denotes the single commit 'r1'. -The '{caret}-' notation includes '' but excludes the th +The '{caret}-[]' notation includes '' but excludes the th parent (i.e. a shorthand for '{caret}..'), with '' = 1 if not given. This is typically useful for merge commits where you can just pass '{caret}-' to get all the commits in the branch -- 2.21.0.1049.geb646f7864
[PATCH v3 0/4] cleanup revisions.txt
Thanks again for the comments, Andreas! I've incorporated all of them into this reroll. --- Changes since v2: * Marked more optional arguments with [] * Added Andreas' "revisions.txt: remove ambibuity between : and :" patch Changes since v1: * Added patch to fix instances of "rev" to "" * Marked all optional rev arguments with [] Andreas Heiduk (1): revisions.txt: remove ambibuity between : and : Denton Liu (3): revisions.txt: change "rev" to "" revisions.txt: mark optional rev arguments with [] revisions.txt: mention ~ form Documentation/revisions.txt | 29 ++--- 1 file changed, 14 insertions(+), 15 deletions(-) -- 2.21.0.1049.geb646f7864
[PATCH v3 1/4] revisions.txt: change "rev" to ""
In revisions.txt, there were some instances of a rev argument being written as "rev". However, since they didn't mean the string literal, write "", instead. Signed-off-by: Denton Liu --- Documentation/revisions.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 2337a995ec..e5f11691b1 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -159,12 +159,12 @@ thing no matter the case. '{caret}0' is a short-hand for '{caret}\{commit\}'. + -'rev{caret}\{object\}' can be used to make sure 'rev' names an -object that exists, without requiring 'rev' to be a tag, and -without dereferencing 'rev'; because a tag is already an object, +'{caret}\{object\}' can be used to make sure '' names an +object that exists, without requiring '' to be a tag, and +without dereferencing ''; because a tag is already an object, it does not have to be dereferenced even once to get to an object. + -'rev{caret}\{tag\}' can be used to ensure that 'rev' identifies an +'{caret}\{tag\}' can be used to ensure that '' identifies an existing tag object. '{caret}{}', e.g. 'v0.99.8{caret}{}':: -- 2.21.0.1049.geb646f7864
[PATCH v3 3/4] revisions.txt: mention ~ form
In revisions.txt, the '^' form is mentioned but the '~' form is missing. Although both forms are essentially equivalent (they each get the first parent of the specified revision), we should mention the latter for completeness. Make this change. Signed-off-by: Denton Liu --- Documentation/revisions.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index dd99770c47..a38ec7fb52 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -139,7 +139,9 @@ thing no matter the case. '{caret}0' means the commit itself and is used when '' is the object name of a tag object that refers to a commit object. -'{tilde}', e.g. 'master{tilde}3':: +'{tilde}[]', e.g. 'HEAD{tilde}, master{tilde}3':: + A suffix '{tilde}' to a revision parameter means the first parent of + that commit object. A suffix '{tilde}' to a revision parameter means the commit object that is the th generation ancestor of the named commit object, following only the first parents. I.e. '{tilde}3' is -- 2.21.0.1049.geb646f7864
[PATCH v3 4/4] revisions.txt: remove ambibuity between : and :
From: Andreas Heiduk The revision ':README' is mentioned as an example for ':' but the explanation forwards to the '::' syntax. At the same time '::' did not mark the ':' as optional. Signed-off-by: Andreas Heiduk Signed-off-by: Denton Liu --- Just for clarification, the correct procedure is to include my sign-off on top of Andreas' when I'm forwarding patches, correct? If it's not, Junio, please take my sign-off for me. Documentation/revisions.txt | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index a38ec7fb52..82c1e5754e 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -196,19 +196,16 @@ existing tag object. Depending on the given text, the shell's word splitting rules might require additional quoting. -':', e.g. 'HEAD:README', ':README', 'master:./README':: +':', e.g. 'HEAD:README', 'master:./README':: A suffix ':' followed by a path names the blob or tree at the given path in the tree-ish object named by the part before the colon. - ':path' (with an empty part before the colon) - is a special case of the syntax described next: content - recorded in the index at the given path. A path starting with './' or '../' is relative to the current working directory. The given path will be converted to be relative to the working tree's root directory. This is most useful to address a blob or tree from a commit or tree that has the same tree structure as the working tree. -'::', e.g. ':0:README', ':README':: +':[:]', e.g. ':0:README', ':README':: A colon, optionally followed by a stage number (0 to 3) and a colon, followed by a path, names a blob object in the index at the given path. A missing stage number (and the colon -- 2.21.0.1049.geb646f7864
Re: [PATCH v3 0/4] cleanup revisions.txt
Sorry, I specified the wrong --in-reply-to. The correct parent message should be this[1]. [1]: https://public-inbox.org/git/cover.1556367012.git.liu.den...@gmail.com/ On Sun, May 05, 2019 at 12:06:54PM -0400, Denton Liu wrote: > Thanks again for the comments, Andreas! I've incorporated all of them > into this reroll. > > --- > > Changes since v2: > > * Marked more optional arguments with [] > * Added Andreas' "revisions.txt: remove ambibuity between : > and :" patch > > Changes since v1: > > * Added patch to fix instances of "rev" to "" > * Marked all optional rev arguments with [] > > > Andreas Heiduk (1): > revisions.txt: remove ambibuity between : and : > > Denton Liu (3): > revisions.txt: change "rev" to "" > revisions.txt: mark optional rev arguments with [] > revisions.txt: mention ~ form > > Documentation/revisions.txt | 29 ++--- > 1 file changed, 14 insertions(+), 15 deletions(-) > > -- > 2.21.0.1049.geb646f7864 >
[PATCH 1/7] t4014: clean up style
In Git's tests, there is typically no space between the redirection operator and the filename. Remove these spaces. Since output is silenced when running without `-v` and debugging output is useful with `-v`, remove redirections to /dev/null. Change here-docs from `<<\EOF` to `<<-\EOF` so that they can be indented along with the rest of the test case. Finally, refactor to remove Git commands upstream of pipe. This way, if an invocation of a Git command fails, the return code won't be lost. Keep upstream non-Git commands since we have to assume a base level of sanity. Signed-off-by: Denton Liu --- t/t4014-format-patch.sh | 614 +--- 1 file changed, 319 insertions(+), 295 deletions(-) diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh index 909c743c13..d05cd256c7 100755 --- a/t/t4014-format-patch.sh +++ b/t/t4014-format-patch.sh @@ -34,7 +34,8 @@ test_expect_success setup ' git commit -m "Side changes #3 with \\n backslash-n in it." && git checkout master && - git diff-tree -p C2 | git apply --index && + git diff-tree -p C2 >patch && + git apply --index patch && grep "^From " patch | wc -l) && test $cnt = 3 ' @@ -85,21 +86,22 @@ test_expect_success "format-patch result applies" ' git checkout -b rebuild-0 master && git am -3 patch0 && - cnt=$(git rev-list master.. | wc -l) && - test $cnt = 2 + git rev-list master.. >list && + test_line_count = 2 list ' test_expect_success "format-patch --ignore-if-in-upstream result applies" ' git checkout -b rebuild-1 master && git am -3 patch1 && - cnt=$(git rev-list master.. | wc -l) && - test $cnt = 2 + git rev-list master.. >list && + test_line_count = 2 list ' test_expect_success 'commit did not screw up the log message' ' - git cat-file commit side | grep "^Side .* with .* backslash-n" + git cat-file commit side >actual && + grep "^Side .* with .* backslash-n" actual ' @@ -112,7 +114,8 @@ test_expect_success 'format-patch did not screw up the log message' ' test_expect_success 'replay did not screw up the log message' ' - git cat-file commit rebuild-1 | grep "^Side .* with .* backslash-n" + git cat-file commit rebuild-1 >actual && + grep "^Side .* with .* backslash-n" actual ' @@ -122,8 +125,8 @@ test_expect_success 'extra headers' ' " && git config --add format.headers "Cc: S E Cipient " && - git format-patch --stdout master..side > patch2 && - sed -e "/^\$/q" patch2 > hdrs2 && + git format-patch --stdout master..side >patch2 && + sed -e "/^\$/q" patch2 >hdrs2 && grep "^To: R E Cipient \$" hdrs2 && grep "^Cc: S E Cipient \$" hdrs2 @@ -134,7 +137,7 @@ test_expect_success 'extra headers without newlines' ' git config --replace-all format.headers "To: R E Cipient " && git config --add format.headers "Cc: S E Cipient " && git format-patch --stdout master..side >patch3 && - sed -e "/^\$/q" patch3 > hdrs3 && + sed -e "/^\$/q" patch3 >hdrs3 && grep "^To: R E Cipient \$" hdrs3 && grep "^Cc: S E Cipient \$" hdrs3 @@ -144,8 +147,8 @@ test_expect_success 'extra headers with multiple To:s' ' git config --replace-all format.headers "To: R E Cipient " && git config --add format.headers "To: S E Cipient " && - git format-patch --stdout master..side > patch4 && - sed -e "/^\$/q" patch4 > hdrs4 && + git format-patch --stdout master..side >patch4 && + sed -e "/^\$/q" patch4 >hdrs4 && grep "^To: R E Cipient ,\$" hdrs4 && grep "^ *S E Cipient \$" hdrs4 ' @@ -153,72 +156,82 @@ test_expect_success 'extra headers with multiple To:s' ' test_expect_success 'additional command line cc (ascii)' ' git config --replace-all format.headers "Cc: R E Cipient " && - git format-patch --cc="S E Cipient " --stdout master..side | sed -e "/^\$/q" >patch5 && - grep "^Cc: R E Cipient ,\$" patch5 && - grep "^ *S E Cipient \$" patch5 + git format-patch --cc="S E Cipient " --stdout master..side >patch5 && + sed -e "/^\$/q" patch5 >hdrs5 && + grep "^Cc: R E Cipient ,\$" hdrs5 && + grep "^ *S E Cipient \$" hdrs5 ' test_expect_failure 'additional command line cc (rfc822)' ' git config --replace-all format.headers "Cc: R E Cipient " && - git format-patch --cc="S. E. Cipient " --stdout master..side | sed -e "/^\$/q" >patch5 && - grep "^Cc: R E Cipient ,\$" patch5 && - grep "^ *\"S. E. Cipient\" \$" patch5 + git format-patch --cc="S. E. Cipient " --stdout master..side >patch5 && + sed -e "/^\$/q" patch5 >hdrs5 && + grep "^Cc: R E Cipient ,\$" hdrs5 && + grep "^ *\"S. E. Cipient\" \$" hdrs5 ' test_expect_success 'command line headers' ' git config --unset-all format.headers && -
[PATCH 0/7] teach branch-specific options for format-patch
Currently, format-patch only accepts branch..description as a branch-specific configuration variable. However, there are many other options which would be useful to have on a branch-by-branch basis, namely cover letter subject and To: and Cc: headers. Teach format-patch to recognise these branch-specific configuration options. Note that this patchset[1] was created using these new configuration options: [branch "submitted/fix-revisions-txt"] coverSubject = "cleanup revisions.txt" cc = "Andreas Heiduk " cc = "Duy Nguyen " cc = "Junio C Hamano " So this is a live example of this patchset working in practice. [1]: https://public-inbox.org/git/cover.1557072286.git.liu.den...@gmail.com/ Denton Liu (7): t4014: clean up style Doc: add more detail for git-format-patch branch.c: extract read_branch_config function format-patch: make cover letter subject configurable format-patch: move extra_headers logic later string-list: create string_list_append_all format-patch: read branch-specific To: and Cc: headers Documentation/config/branch.txt| 10 + Documentation/git-format-patch.txt | 33 +- branch.c | 14 +- branch.h | 5 + builtin/log.c | 147 -- string-list.c | 9 + string-list.h | 7 + t/t4014-format-patch.sh| 708 + t/t9902-completion.sh | 5 +- 9 files changed, 590 insertions(+), 348 deletions(-) -- 2.21.0.1049.geb646f7864
[PATCH 7/7] format-patch: read branch-specific To: and Cc: headers
If a user wishes to keep track of whom to Cc: on individual patchsets, they must manually keep track of each recipient and fill it in with the `--cc` option on git-format-patch each time. However, on the Git mailing list, Cc:'s are typically never dropped. As a result, it would be nice to have a method to keep track of recipients on a per-branch basis. Currently, git-format-patch gets its To: headers from the `--to` options and the `format.to` config variable. The Cc: header is derived similarly. In addition to the above, read To: and Cc: headers from `branch..to` and `branch..cc` so that users can have branch-specific configuration options. Signed-off-by: Denton Liu --- Documentation/config/branch.txt| 6 ++ Documentation/git-format-patch.txt | 10 +-- builtin/log.c | 63 +++-- t/t4014-format-patch.sh| 108 +++-- 4 files changed, 159 insertions(+), 28 deletions(-) diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt index 2bff738982..22b9bf3d0d 100644 --- a/Documentation/config/branch.txt +++ b/Documentation/config/branch.txt @@ -104,3 +104,9 @@ branch..description:: branch..coverSubject:: When format-patch generates a cover letter, use the specified subject for the cover letter instead of the generic template. + +branch..to:: +branch..cc:: + Additional recipients to include in a patch to be submitted + by mail. See the --to and --cc options in + linkgit:git-format-patch[1]. diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index af7883acbe..1c972f683a 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -199,7 +199,7 @@ will want to ensure that threading is disabled for `git send-email`. Add a `To:` header to the email headers. This is in addition to any configured headers, and may be used multiple times. The emails given will be used along with any emails given by - `format.to` configurations. + `format.to` and `branch..to` configurations. The negated form `--no-to` discards all `To:` headers added so far (from config or command line). @@ -207,7 +207,7 @@ will want to ensure that threading is disabled for `git send-email`. Add a `Cc:` header to the email headers. This is in addition to any configured headers, and may be used multiple times. The emails given will be used along with any emails given by - `format.cc` configurations. + `format.cc` and `branch..cc` configurations. The negated form `--no-cc` discards all `Cc:` headers added so far (from config or command line). @@ -335,7 +335,7 @@ CONFIGURATION - You can specify extra mail header lines to be added to each message, defaults for the subject prefix and file suffix, number patches when -outputting more than one patch, add "To" or "Cc:" headers, configure +outputting more than one patch, add "To:" or "Cc:" headers, configure attachments, and sign off patches with configuration variables. @@ -352,11 +352,13 @@ attachments, and sign off patches with configuration variables. In addition, for a specific branch, you can specify a custom cover -letter subject. +letter subject, and add additional "To:" or "Cc:" headers. [branch "branch-name"] coverSubject = "subject for branch-name only" + to = + cc = DISCUSSION diff --git a/builtin/log.c b/builtin/log.c index 685e319078..6825d95c5f 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -738,6 +738,8 @@ static char *default_attach = NULL; static struct string_list extra_hdr = STRING_LIST_INIT_NODUP; static struct string_list extra_to = STRING_LIST_INIT_NODUP; static struct string_list extra_cc = STRING_LIST_INIT_NODUP; +int to_cleared; +int cc_cleared; static void add_header(const char *value) { @@ -1030,6 +1032,55 @@ static void show_diffstat(struct rev_info *rev, fprintf(rev->diffopt.file, "\n"); } +static void add_branch_headers(struct rev_info *rev, const char *branch_name) +{ + struct strbuf buf = STRBUF_INIT; + const struct string_list *values; + + if (!branch_name) + branch_name = find_branch_name(rev); + + if (!branch_name || !*branch_name) + return; + + /* +* HACK: We only use branch-specific recipients iff the list has not +* been cleared by an earlier --no-{to,cc} option on the command-line. +* +* When we get format.{to,cc} options, they can be cleared by +* --no-{to,cc} options since the `git_config` call comes before the +* `parse_options` call. +* +* However, in the case of branch..{to,cc}, this function needs +* to be called after `setup_revisions`, which must be called after +* `parse_options`. However, in or
[PATCH 3/7] branch.c: extract read_branch_config function
In the future, we'll need to use `read_branch_config` as a generic base for other branch-config reading functions. Extract it from `read_branch_desc` so that it can be reused later. Signed-off-by: Denton Liu --- branch.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/branch.c b/branch.c index 28b81a7e02..4b49976924 100644 --- a/branch.c +++ b/branch.c @@ -162,11 +162,11 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, free(tracking.src); } -int read_branch_desc(struct strbuf *buf, const char *branch_name) +static int read_branch_config(struct strbuf *buf, const char *branch_name, const char *key) { char *v = NULL; struct strbuf name = STRBUF_INIT; - strbuf_addf(&name, "branch.%s.description", branch_name); + strbuf_addf(&name, "branch.%s.%s", branch_name, key); if (git_config_get_string(name.buf, &v)) { strbuf_release(&name); return -1; @@ -177,6 +177,11 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name) return 0; } +int read_branch_desc(struct strbuf *buf, const char *branch_name) +{ + return read_branch_config(buf, branch_name, "description"); +} + /* * Check if 'name' can be a valid name for a branch; die otherwise. * Return 1 if the named branch already exists; return 0 otherwise. -- 2.21.0.1049.geb646f7864
[PATCH 2/7] Doc: add more detail for git-format-patch
In git-format-patch.txt, we were missing some key user information. First of all, using the `--to` and `--cc` options don't override `format.to` and `format.cc` variables, respectively. They add on to each other. Document this. In addition, document the special value of `--base=auto`. Finally, while we're at it, surround option arguments with <>. Signed-off-by: Denton Liu --- Documentation/git-format-patch.txt | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 1af85d404f..7b71d4e2ed 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -17,9 +17,9 @@ SYNOPSIS [--signature-file=] [-n | --numbered | -N | --no-numbered] [--start-number ] [--numbered-files] - [--in-reply-to=Message-Id] [--suffix=.] + [--in-reply-to=] [--suffix=.] [--ignore-if-in-upstream] - [--rfc] [--subject-prefix=Subject-Prefix] + [--rfc] [--subject-prefix=] [(--reroll-count|-v) ] [--to=] [--cc=] [--[no-]cover-letter] [--quiet] [--notes[=]] @@ -158,7 +158,7 @@ Beware that the default for 'git send-email' is to thread emails itself. If you want `git format-patch` to take care of threading, you will want to ensure that threading is disabled for `git send-email`. ---in-reply-to=Message-Id:: +--in-reply-to=:: Make the first mail (or all the mails with `--no-thread`) appear as a reply to the given Message-Id, which avoids breaking threads to provide a new patch series. @@ -192,13 +192,17 @@ will want to ensure that threading is disabled for `git send-email`. --to=:: Add a `To:` header to the email headers. This is in addition - to any configured headers, and may be used multiple times. + to any configured headers, and may be used multiple times. The + emails given will be used along with any emails given by + `format.to` configurations. The negated form `--no-to` discards all `To:` headers added so far (from config or command line). --cc=:: Add a `Cc:` header to the email headers. This is in addition - to any configured headers, and may be used multiple times. + to any configured headers, and may be used multiple times. The + emails given will be used along with any emails given by + `format.cc` configurations. The negated form `--no-cc` discards all `Cc:` headers added so far (from config or command line). @@ -309,7 +313,8 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`. --base=:: Record the base tree information to identify the state the patch series applies to. See the BASE TREE INFORMATION section - below for details. + below for details. If is equal to "auto", a base commit + is automatically chosen. --root:: Treat the revision argument as a , even if it -- 2.21.0.1049.geb646f7864
[PATCH 5/7] format-patch: move extra_headers logic later
In a future patch, we need to perform the addition of To: and Cc: to extra_headers after the branch_name logic. Simply transpose this logic later in the function so that this happens. (This patch is best viewed with `git diff --color-moved`.) Note that this logic only depends on the `git_config` and `repo_init_revisions` and is depended on by the patch creation logic which is directly below it so this move is effectively a no-op as no dependencies being reordered. Signed-off-by: Denton Liu --- builtin/log.c | 58 +-- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 6f19326aea..685e319078 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1665,35 +1665,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.subject_prefix = strbuf_detach(&sprefix, NULL); } - for (i = 0; i < extra_hdr.nr; i++) { - strbuf_addstr(&buf, extra_hdr.items[i].string); - strbuf_addch(&buf, '\n'); - } - - if (extra_to.nr) - strbuf_addstr(&buf, "To: "); - for (i = 0; i < extra_to.nr; i++) { - if (i) - strbuf_addstr(&buf, ""); - strbuf_addstr(&buf, extra_to.items[i].string); - if (i + 1 < extra_to.nr) - strbuf_addch(&buf, ','); - strbuf_addch(&buf, '\n'); - } - - if (extra_cc.nr) - strbuf_addstr(&buf, "Cc: "); - for (i = 0; i < extra_cc.nr; i++) { - if (i) - strbuf_addstr(&buf, ""); - strbuf_addstr(&buf, extra_cc.items[i].string); - if (i + 1 < extra_cc.nr) - strbuf_addch(&buf, ','); - strbuf_addch(&buf, '\n'); - } - - rev.extra_headers = strbuf_detach(&buf, NULL); - if (from) { if (split_ident_line(&rev.from_ident, from, strlen(from))) die(_("invalid ident line: %s"), from); @@ -1796,6 +1767,35 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) } } + for (i = 0; i < extra_hdr.nr; i++) { + strbuf_addstr(&buf, extra_hdr.items[i].string); + strbuf_addch(&buf, '\n'); + } + + if (extra_to.nr) + strbuf_addstr(&buf, "To: "); + for (i = 0; i < extra_to.nr; i++) { + if (i) + strbuf_addstr(&buf, ""); + strbuf_addstr(&buf, extra_to.items[i].string); + if (i + 1 < extra_to.nr) + strbuf_addch(&buf, ','); + strbuf_addch(&buf, '\n'); + } + + if (extra_cc.nr) + strbuf_addstr(&buf, "Cc: "); + for (i = 0; i < extra_cc.nr; i++) { + if (i) + strbuf_addstr(&buf, ""); + strbuf_addstr(&buf, extra_cc.items[i].string); + if (i + 1 < extra_cc.nr) + strbuf_addch(&buf, ','); + strbuf_addch(&buf, '\n'); + } + + rev.extra_headers = strbuf_detach(&buf, NULL); + /* * We cannot move this anywhere earlier because we do want to * know if --root was given explicitly from the command line. -- 2.21.0.1049.geb646f7864
[PATCH 6/7] string-list: create string_list_append_all
In a future patch, we'll need to take one string_list and append it to the end of another. Create the `string_list_append_all` function which does this. Signed-off-by: Denton Liu --- string-list.c | 9 + string-list.h | 7 +++ 2 files changed, 16 insertions(+) diff --git a/string-list.c b/string-list.c index a917955fbd..e63d58fbd2 100644 --- a/string-list.c +++ b/string-list.c @@ -215,6 +215,15 @@ struct string_list_item *string_list_append(struct string_list *list, list->strdup_strings ? xstrdup(string) : (char *)string); } +void string_list_append_all(struct string_list *list, + const struct string_list *append_list) +{ + struct string_list_item *item; + ALLOC_GROW(list->items, list->nr + append_list->nr, list->alloc); + for_each_string_list_item(item, append_list) + string_list_append(list, item->string); +} + /* * Encapsulate the compare function pointer because ISO C99 forbids * casting from void * to a function pointer and vice versa. diff --git a/string-list.h b/string-list.h index 18c718c12c..32e0c4b47f 100644 --- a/string-list.h +++ b/string-list.h @@ -208,6 +208,13 @@ struct string_list_item *string_list_append(struct string_list *list, const char */ struct string_list_item *string_list_append_nodup(struct string_list *list, char *string); +/** + * Add all strings in append_list to list. If list->strdup_string is + * set, then each string is copied; otherwise the new string_list_entry + * refers to the entry in the append_list. + */ +void string_list_append_all(struct string_list *list, const struct string_list *append_list); + /** * Sort the list's entries by string value in `strcmp()` order. */ -- 2.21.0.1049.geb646f7864
[PATCH 4/7] format-patch: make cover letter subject configurable
We used to populate the subject of the cover letter generated by git-format-patch with "*** SUBJECT HERE ***". However, if a user submits multiple patchsets, they may want to keep a consistent subject between rerolls. If git-format-patch is run on a branch that has `branch..coverSubject` defined, make the cover letter's subject be that value instead of the generic "*** SUBJECT HERE ***". In addition, add the `--cover-subject` option to override this setting. Signed-off-by: Denton Liu --- Documentation/config/branch.txt| 4 Documentation/git-format-patch.txt | 12 branch.c | 5 + branch.h | 5 + builtin/log.c | 26 +++--- t/t4014-format-patch.sh| 20 t/t9902-completion.sh | 5 - 7 files changed, 69 insertions(+), 8 deletions(-) diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt index 019d60ede2..2bff738982 100644 --- a/Documentation/config/branch.txt +++ b/Documentation/config/branch.txt @@ -100,3 +100,7 @@ branch..description:: `git branch --edit-description`. Branch description is automatically added in the format-patch cover letter or request-pull summary. + +branch..coverSubject:: + When format-patch generates a cover letter, use the specified + subject for the cover letter instead of the generic template. diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 7b71d4e2ed..af7883acbe 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -19,6 +19,7 @@ SYNOPSIS [--start-number ] [--numbered-files] [--in-reply-to=] [--suffix=.] [--ignore-if-in-upstream] + [--cover-subject=] [--rfc] [--subject-prefix=] [(--reroll-count|-v) ] [--to=] [--cc=] @@ -170,6 +171,10 @@ will want to ensure that threading is disabled for `git send-email`. patches being generated, and any patch that matches is ignored. +--cover-subject=:: + Instead of using the default "*** SUBJECT HERE ***" subject for + the cover letter, use the given . + --subject-prefix=:: Instead of the standard '[PATCH]' prefix in the subject line, instead use '[]'. This @@ -346,6 +351,13 @@ attachments, and sign off patches with configuration variables. coverletter = auto +In addition, for a specific branch, you can specify a custom cover +letter subject. + + +[branch "branch-name"] + coverSubject = "subject for branch-name only" + DISCUSSION -- diff --git a/branch.c b/branch.c index 4b49976924..40d30b8fa7 100644 --- a/branch.c +++ b/branch.c @@ -182,6 +182,11 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name) return read_branch_config(buf, branch_name, "description"); } +int read_branch_subject(struct strbuf *buf, const char *branch_name) +{ + return read_branch_config(buf, branch_name, "coversubject"); +} + /* * Check if 'name' can be a valid name for a branch; die otherwise. * Return 1 if the named branch already exists; return 0 otherwise. diff --git a/branch.h b/branch.h index 29c1afa4d0..6a8936bbc8 100644 --- a/branch.h +++ b/branch.h @@ -79,6 +79,11 @@ extern int install_branch_config(int flag, const char *local, const char *origin */ extern int read_branch_desc(struct strbuf *, const char *branch_name); +/* + * Read branch subject + */ +extern int read_branch_subject(struct strbuf *, const char *branch_name); + /* * Check if a branch is checked out in the main worktree or any linked * worktree and die (with a message describing its checkout location) if diff --git a/builtin/log.c b/builtin/log.c index ab859f5904..6f19326aea 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1034,13 +1034,14 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, struct commit *origin, int nr, struct commit **list, const char *branch_name, + const char *subject, int quiet) { const char *committer; - const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n"; - const char *msg; + const char *body = "*** BLURB HERE ***"; struct shortlog log; struct strbuf sb = STRBUF_INIT; + struct strbuf subject_sb = STRBUF_INIT; int i; const char *encoding = "UTF-8"; int need_8bit_cte = 0; @@ -1068,17 +1069,24 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, if (!branch_name) branch_name = find_branch_name(rev); - msg = body; + if (!subject) { + if (branch_name && *b
Re: Contributing with documentation/translation
On Wednesday, 1 May 2019 22:35:45 CEST Jeff King wrote: > On Wed, May 01, 2019 at 05:11:38PM +0700, Duy Nguyen wrote: > > > On Wed, May 1, 2019 at 1:10 AM Priscila Gutierres > > wrote: > > > > > > Hi > > > I want to contribute to git by creating and/or translating > > > documentation. Where may I find the info to do this? > > > > For translation, you could start at po/README. That's mostly UI > > translation. I think some team actually started document translation > > too [1], but I don't know the status. > > +cc Jean-Noel, who is working on this. It looks like translations will > be done through Weblate: > > https://hosted.weblate.org/projects/git-manpages/translations/ > > There's some more discussion in: > > https://public-inbox.org/git/1992944.NOdEsaAZKb@cayenne/ > > There are also translations of the "Pro Git" book. This isn't strictly > "Git documentation", but it is CC-licensed and we show it on > git-scm.com. Details are at: > > https://github.com/progit/progit2/blob/master/TRANSLATING.md > > -Peff > Hi Priscila, Peff has said it all. Depending on where you are on you way to learning about git, I would recommend: * Starting with Progit for a gentle introduction to using git * Translating git manpages for extensive knowledge of all git commands * Translating git itself for indepth knowledge of how git works The translation teams are welcoming to new contributors. Jean-Noël
Re: [PATCH v3 0/4] remove extern from function declarations
On Sat, May 4, 2019 at 10:28 PM Junio C Hamano wrote: > > Jeff King writes: > > > I think spatch is smart enough not to hit the same header multiple > > times. But the problem is that we invoke it once per file, so it > > actually processes cache.h many times. That's slow, but also produces > > bogus patches. > > Yes, I've seen this and was a bit irritated myself, but not enough > to do something about it myself, yet. > > > > Jacob Keller's patches to collapse this to a single invocation do fix it > > (and I've used them selectively in the past rather than cleaning up the > > resulting patch manually ;) ). > > Ah, that is nice to know. Do we want to resurrect it? I remember deciding that the memory cost trade off was too high back then. Thanks, Jake
Re: [PATCH v3 0/4] remove extern from function declarations
On Fri, May 3, 2019 at 11:09 AM Jeff King wrote: > > On Fri, May 03, 2019 at 04:42:11PM +0200, SZEDER Gábor wrote: > > > > Since you *could* include it, I now assume that Coccinelle does not need > > > to follow the `#include`s (otherwise, it would have complained about not > > > finding the `windows.h` header in your setup). > > > > We invoke Coccinelle/spatch with the '--all-includes' option, see the > > SPATCH_FLAGS make variable. And it does indeed include header files: > > I've seen it find something to transform in 'cache.h', and then the > > resulting 'contrib/coccinelle/.cocci.patch' included the > > exact same transformation as many times as the number of files > > including 'cache.h'... which is a lot :) > > I think spatch is smart enough not to hit the same header multiple > times. But the problem is that we invoke it once per file, so it > actually processes cache.h many times. That's slow, but also produces > bogus patches. > > Jacob Keller's patches to collapse this to a single invocation do fix it > (and I've used them selectively in the past rather than cleaning up the > resulting patch manually ;) ). > > Sort of tangential to your point, I guess, but it might be a topic to > revisit. > > -Peff Yea, my patches are much faster. However, they trade off a huge increase in memory consumption, and from what I remember, the failure if you don't have enough RAM was not a good experience. Thanks, Jake
Re: git has issues with international characters in branch names
On Sun, May 05, 2019 at 12:27:31PM +0200, Ax Da wrote: > The documentation doesn't seem to be clear about which characters are allowed > as branch names. > > We have been using umlauts and apostrophes in our branch names, and both are > having issues: > > - apostrophe: > git removes the apostrophes from the branch name when creating/pushing a > branch. > In our TFS server repository we're having the same branch name twice: Once > with apostrophes, > and once without. And I don't seem to be able to delete the one without > apostrophes. > > - umlauts: > The umlauts are not correctly interpreted when SMB is used. "gemäss" > becomes "gemss". > (See images attached.) I don't think Git itself has a problem with Unicode. I literally just the other day created a branch with a Unicode apostrophe and pushed it to GitHub successfully using macOS. It is possible that TFS doesn't like them; that depends on the hosting solution you use. If you're seeing problems with TFS, I'd reach out to Microsoft for assistance. It looks also like the Git CMD interface isn't rendering them properly. I would try using Git Bash instead, and if that doesn't work, please report that to Git for Windows. The folks there will have a better idea about the portability issue that's occurring; I expect there's a wart there somewhere between UTF-8 and UTF-16. It may also be that SMB is not a good choice for sharing repositories if you require internationalization, or you may need to change the character set your SMB server uses. You may also, depending on which version of Windows you are using, have better luck at the command line with Windows Subsystem for Linux. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Git build on antique PowerMac
Hi Everyone, I have a PowerMac I use for testing. It provides several testing differentiators, like OS X 10.5, Bash 3.2, GCC 4.0.1, Apple cc-tools linker, and big-endian PowerPC. (I think Gentoo provides a Linux image for the hardware, but I don't use it). The Git libraries and programs build fine out of the box. They also seem to work as expected once installed. The pain point is the self self tests, which I have never been able to build (or execute). I'd like to close this gap. make -C templates SHELL_PATH='/bin/sh' PERL_PATH='/usr/bin/perl' : no custom templates yet make -C t/ all rm -f -r 'test-results' readline() on unopened filehandle test_must_fail run_sub_test_lib_test at check-non-portable-shell.pl line 34. Modification of a read-only value attempted at check-non-portable-shell.pl line 34. make[1]: *** [test-lint-shell-syntax] Error 255 Does anyone want to take a shot at this antique system? I can provide SSH access with root. Jeff
Surprising semantics of "git add file"
Hi! Reading the manual, I realized that "it is the way it is (it works as documented)", but to me it's surprising, meaning: I feel it's wrong: (git-2.16.4) I have staged several files, and then I do "git add -interactive file" to add some selected changes from file. After that a "git diff --cached file" just shows the changes added interactively, but when I "git add file" to commit those changes), even the unstaged changes from file are committed. I feel this is inconsistent: At least "git diff --cached file" should behave like "git commit file", meaning "git commit file" should be fixed IMHO. Or are there any reasonable use cased for that? Regards, Ulrich Windl
Antw: Surprising semantics of "git add file"
Actually things are worse: When I tried to fix the comment for the unexpected commit at least using "git commit --amend", more files were committed! >>> Ulrich Windl 05.05.19 22.02 Uhr >>> Hi! Reading the manual, I realized that "it is the way it is (it works as documented)", but to me it's surprising, meaning: I feel it's wrong: (git-2.16.4) I have staged several files, and then I do "git add -interactive file" to add some selected changes from file. After that a "git diff --cached file" just shows the changes added interactively, but when I "git add file" to commit those changes), even the unstaged changes from file are committed. I feel this is inconsistent: At least "git diff --cached file" should behave like "git commit file", meaning "git commit file" should be fixed IMHO. Or are there any reasonable use cased for that? Regards, Ulrich Windl
Re: Git build on antique PowerMac
On Sun, May 05, 2019 at 03:42:45PM -0400, Jeffrey Walton wrote: > I have a PowerMac I use for testing. It provides several testing > differentiators, like OS X 10.5, Bash 3.2, GCC 4.0.1, Apple cc-tools > linker, and big-endian PowerPC. (I think Gentoo provides a Linux image > for the hardware, but I don't use it). > > The Git libraries and programs build fine out of the box. They also > seem to work as expected once installed. > > The pain point is the self self tests, which I have never been able to > build (or execute). I'd like to close this gap. > > make -C templates SHELL_PATH='/bin/sh' PERL_PATH='/usr/bin/perl' > : no custom templates yet > make -C t/ all > rm -f -r 'test-results' > readline() on unopened filehandle test_must_fail > run_sub_test_lib_test at check-non-portable-shell.pl line 34. > Modification of a read-only value attempted at > check-non-portable-shell.pl line 34. > make[1]: *** [test-lint-shell-syntax] Error 255 You could try: make -C t all TEST_LINT= That error comes from one of the "lint" targets that are supposed to catch some common errors that we repeatedly made in our tests. This linter is enabled by default, but this command disables it. Yeah, it's not the proper solution, but I would think that you don't need them on that antique PowerMac anyway.
Re: Surprising semantics of "git add file"
On Mai 05 2019, "Ulrich Windl" wrote: > After that a "git diff --cached file" just shows the changes added > interactively, but when I "git add file" to commit those changes), even the > unstaged changes from file are committed. Did you really mean "git add" here? It doesn't commit, it adds to the index. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: Git build on antique PowerMac
On Sun, May 5, 2019 at 3:43 PM Jeffrey Walton wrote: > I have a PowerMac I use for testing. [...] > make -C t/ all > readline() on unopened filehandle test_must_fail > run_sub_test_lib_test at check-non-portable-shell.pl line 34. > Modification of a read-only value attempted at > check-non-portable-shell.pl line 34. > make[1]: *** [test-lint-shell-syntax] Error 255 > > Does anyone want to take a shot at this antique system? I can provide > SSH access with root. As the author of the code triggering that problem, I can take a look at it. (You already have my public SSH key on your Solaris box.) Given [1], I can see (I guess) why it's complaining about modification of a read-only variable. However, the unopened filehandle complaint looks odd. [1]: https://www.perlmonks.org/?node_id=570712#default_unlocalized
deal
-- Dear Friend, Greetings and how are you today? I'm sorry if this message is found in the spam of your email box as that will certainly be a network mishap and I want us to discuss about the sum of Forty Million Dollars discovered in my office and I'll give you more details when I hear from you. Regards Jean Kabore --
You emailed Babel No More
Name: 办#發#票#,微-电/微:134﹣1861,89,58,张 Email: git@vger.kernel.org Comments: 办#發#票#,微-电/微:134﹣1861,89,58,张
Re: git has issues with international characters in branch names
On Sun, May 5, 2019 at 11:16 AM brian m. carlson wrote: > > On Sun, May 05, 2019 at 12:27:31PM +0200, Ax Da wrote: > > The documentation doesn't seem to be clear about which characters are > > allowed as branch names. > > > > We have been using umlauts and apostrophes in our branch names, and both > > are having issues: > > > > - apostrophe: > > git removes the apostrophes from the branch name when creating/pushing a > > branch. > > In our TFS server repository we're having the same branch name twice: > > Once with apostrophes, > > and once without. And I don't seem to be able to delete the one without > > apostrophes. > > > > - umlauts: > > The umlauts are not correctly interpreted when SMB is used. "gemäss" > > becomes "gemss". > > (See images attached.) > > I don't think Git itself has a problem with Unicode. I literally just > the other day created a branch with a Unicode apostrophe and pushed it > to GitHub successfully using macOS. It is possible that TFS doesn't like > them; that depends on the hosting solution you use. If you're seeing > problems with TFS, I'd reach out to Microsoft for assistance. > > It looks also like the Git CMD interface isn't rendering them properly. > I would try using Git Bash instead, and if that doesn't work, please > report that to Git for Windows. The folks there will have a better idea > about the portability issue that's occurring; I expect there's a wart > there somewhere between UTF-8 and UTF-16. It may also be that SMB is not > a good choice for sharing repositories if you require > internationalization, or you may need to change the character set your > SMB server uses. In my experience, the regular command prompt will have significant troubles displaying UTF-8 characters. I had better luck in Git Bash, but I don't recall if I had to do something for it. Thanks, Jake > > You may also, depending on which version of Windows you are using, have > better luck at the command line with Windows Subsystem for Linux. > -- > brian m. carlson: Houston, Texas, US > OpenPGP: https://keybase.io/bk2204
Re: [PATCH 1/2] archive: replace write_or_die() calls with write_block_or_die()
On Sun, May 05, 2019 at 02:25:59PM +0900, Junio C Hamano wrote: > Jeff King writes: > > > FWIW, I agree with you here. These patches are not making anything worse > > (and may even make them better, since we'd probably need to swap out the > > BLOCKSIZE constant for a run-time "blocksize" variable in fewer places). > > It's just that leaving the interface uneven is an easy way to > introduce an unnecessary bug, e.g. > > -type function(args) { > +type function(args, size_t blocksize) { > decls; > - helper_one(BLOCKSIZE, other, args); > + helper_one(blocksize, other, args); > helper_two(its, args); > - helper_three(BLOCKSIZE, even, more, args); > + helper_three(blocksize, even, more, args); >} > > when this caller is away from the implementation of helper_two() > that hardcodes the assumption that this callchain only uses > BLOCKSIZE and in an implicit way. > > And that can easily be avoided by defensively making helper_two() to > take BLOCKSIZE as an argument as everybody else in the caller does. > > I do not actually care too deeply, though. Hopefully whoever adds > "-b" would be careful enough to follow all callchain, and at least > look at all the callees that are file-scope static, and the one I > have trouble with _is_ a file-scope static. Right, my assumption was that the first step in the conversion would be somebody doing s/BLOCKSIZE/global_blocksize_variable/. But that is just a guess. > Or maybe nobody does "-b", in which case this ticking time bomb will > not trigger, so we'd be OK. Yes. I suspect we're probably going down an unproductive tangent. :) -Peff
[PATCH] coccicheck: optionally process every source file at once
On Sun, May 05, 2019 at 11:08:35AM -0700, Jacob Keller wrote: > > Jacob Keller's patches to collapse this to a single invocation do fix it > > (and I've used them selectively in the past rather than cleaning up the > > resulting patch manually ;) ). > > > > Sort of tangential to your point, I guess, but it might be a topic to > > revisit. > > Yea, my patches are much faster. However, they trade off a huge > increase in memory consumption, and from what I remember, the failure > if you don't have enough RAM was not a good experience. I think there was a suggestion to make it conditional. So maybe something like this? -- >8 -- From: Jacob Keller Subject: [PATCH] coccicheck: optionally process every source file at once make coccicheck is used in order to apply coccinelle semantic patches, and see if any of the transformations found within contrib/coccinelle/ can be applied to the current code base. Provide an option to pass every file to a single invocation of spatch, instead of running spatch once per source file. This reduces the time required to run make coccicheck by a significant amount of time: Prior timing of make coccicheck real6m14.090s user25m2.606s sys 1m22.919s New timing of make coccicheck real1m36.580s user7m55.933s sys 0m18.219s This is nearly a 4x decrease in the time required to run make coccicheck. This is due to the overhead of restarting spatch for every file. By processing all files at once, we can amortize this startup cost across the total number of files, rather than paying it once per file. However, it comes at a cost. The RSS of each spatch process goes from ~50MB to ~1500MB (and peak memory usage may be even higher if make runs multiple rule files in parallel due to "-j"). That's enough to make some systems (like our Travis build!) fail the whole process, or could make things slower due to swapping. So let's make the new behavior optional, and people with a lot of memory can choose to use it. [peff: modified Jacob's patch to make the behavior optional, since people reported complications due to the memory use] Signed-off-by: Jacob Keller Signed-off-by: Jeff King --- The conditional here is implemented as shell. I couldn't find a way to do it readably with a make ifdef, since we're in the middle of a backslash-connected shell command in the recipe. And trying to use $(if) just turned into a mess. But maybe some gnu make hotshot wants to show me how it's done. ;) Makefile | 20 ++-- 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 9f1b6e8926..5870ea200e 100644 --- a/Makefile +++ b/Makefile @@ -1174,8 +1174,11 @@ PTHREAD_CFLAGS = SPARSE_FLAGS ?= SP_EXTRA_FLAGS = -# For the 'coccicheck' target +# For the 'coccicheck' target; set USE_SINGLE_SPATCH to invoke a single spatch +# for all sources, rather than one per source file. That generally runs faster, +# at the cost of using much more peak memory (on the order of 1-2GB). SPATCH_FLAGS = --all-includes --patch . +USE_SINGLE_SPATCH = include config.mak.uname -include config.mak.autogen @@ -2790,11 +2793,16 @@ endif %.cocci.patch: %.cocci $(COCCI_SOURCES) @echo '' SPATCH $<; \ - ret=0; \ - for f in $(COCCI_SOURCES); do \ - $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \ - { ret=$$?; break; }; \ - done >$@+ 2>$@.log; \ + if test -z "$(USE_SINGLE_SPATCH)"; then \ + ret=0; \ + for f in $(COCCI_SOURCES); do \ + $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \ + { ret=$$?; break; }; \ + done >$@+ 2>$@.log; \ + else \ + $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) >$@+ 2>$@.log; \ + ret=$$?; \ + fi; \ if test $$ret != 0; \ then \ cat $@.log; \ -- 2.21.0.1314.g224b191707