Re: [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value
Ann T Ropea writes: > Neither Git nor the user are in need of this (visual) aid anymore, but > we must offer a transition period. > > Also, fix a typo: "abbbreviated" ---> "abbreviated". > > Signed-off-by: Ann T Ropea > --- > v2: rename patch series & focus on removal of ellipses > v3: env var instead of config option, use one-line comments where > appropriate, preserve indent level > v4: improve env var handling (rename, helper func to query, docu) Thanks for sticking with this topic---very much appreciated, as we saw many newcomers get tired of doing repeated polishing and abandon their topic in the past. I have to go offline now, but will comment later when I come back.
Re: [PATCH 1/1] diffcore: add a filter to find a specific blob
Junio C Hamano writes: > If that is the case, shouldn't we make this new mode imply > --full-history to forbid history simplification? "git log" is a > tool to find _an_ explanation of the current state, and the usual > history simplification makes tons of sense there, but blobfind is > run most likely in order to find _all_ mention of the set of blobs > given. One scenario that I think we may want to be careful about is this: ---o---*---*---A*--M*--o---X \ / o---*---o---B where commits marked with '*' has the same blob M:Makefile you are looking for at the same path Makefile, and we start traversal at X with "git log --blobfind=M:Makefile X" (or even with a pathspec, i.e. "git log --blobfind=M:Makefile X -- Makefile). The usual merge simplification rules would say "Ah, M and A are TREESAME so we do not have to look at the side branch that ends at B". If the user is interested in finding all the introduction and the retirement of a specific blob object, we would miss the transition around the '*' on that side branch and ends up finding only the transitions after the fork point where the blob is introduced, and after M where the blob is retired. Another interesting case we may want to be careful is this: ---A*--M*--o---X / ---B* for the same reason. The usual merge simplification rules are designed to come up with _an_ explanation for the state in X, and because M is TREESAME with both A and B, it would pick just one (the first parent) while ignoring the other. Again, that would not be appropriate if the reason why the user is running the command is to find all the introduction and the retirement of an object. It may be worth covering these in the tests (I didn't try to see specifically if the patch has these cases already, as I didn't think of the issue when I responded---sorry about that). Thanks.
Re: [PATCH] merge-recursive: ignore_case shouldn't reject intentional removals
Elijah Newren writes: > But what it really is forced to do is more of a 4-way merge; a good > chunk of its annoying complexity is based around this (undocumented > and unfortunate) reality. It derives from what I consider a simple > design flaw. Yes, and it does not help that it wants to write into the filesystem while it performs the outermost merges. In the ideal world, we should - ask unpack_trees() to do "read-tree -m" without "-u"; - do all the merge-recursive computations in-core and prepare the resulting index, while keeping the current index intact; - compare the current in-core index and the resulting in-core index, and notice the paths that need to be added, updated or removed in the working tree, and ensure that there is no loss of information when the change is reflected to the working tree, e.g. the result wants to create a file where the working tree currently has a directory with non-expendable contents in it, the result wants to remove a file where the working tree file has local modification, etc.; and then finally - carry out the working tree update to make it match what the resulting in-core index says it should look like.
Re: [PATCH 5/5] sha1_file: don't re-scan pack directory for null sha1
Jeff King writes: > So I dunno. That is far from exhaustive, but it does seem like most > cases should assume the presence of the file. You are right. I should have considered that "we got a random object-name looking thing and we do not care if it does not exist" is a very narrow minority case. Most of the object names we deal with come from local repository traversal and unless things like the "fsck-promisor" comes into the picture, we should always have them available locally. > But it may not be that big a deal. For the most part, all bets are off > in a corrupt repo. My main concern is just that we do not want the > corruption to spread or to make it harder for us to recover from (and > that includes allowing us to delete or overwrite other data that would > otherwise be forbidden, which is what's happening in the fetch case). Absolutely. Thanks.
Re: [PATCH 1/1] convert: tighten the safe autocrlf handling
tbo...@web.de writes:
> From: Torsten Bögershausen
>
> When a text file had been commited with CRLF and the file is commited
> again, the CRLF are kept if .gitattributs has "text=auto".
> This is done by analyzing the content of the blob stored in the index:
> If a '\r' is found, Git assumes that the blob was commited with CRLF.
>
> The simple search for a '\r' does not always work as expected:
> A file is encoded in UTF-16 with CRLF and commited. Git treats it as binary.
> Now the content is converted into UTF-8. At the next commit Git treats the
> file as text, the CRLF should be converted into LF, but isn't.
>
> Solution:
Remove this line.
> Replace has_cr_in_index() with has_crlf_in_index(). When no '\r' is found,
> 0 is returned directly, this is the most common case.
> If a '\r' is found, the content is analyzed more deeply.
I may be recalling things incorrectly, but didn't an old version of
the code check CRLF explicitly, unlike the current implementation
that only check CRs?
In any case, I think we have accumulated enough cruft only to work
around the issues caused by "safe" crlf. I moderately strongly
wonder if we should go back and think if that "feature" is adding
much value, and remove it if it is not.
In the meantime, let's queue this fix on top of the "safe crlf
workaround" pile.
Thanks.
>
> Reported-By: Ashish Negi
> Signed-off-by: Torsten Bögershausen
> ---
> convert.c| 19 +
> t/t0027-auto-crlf.sh | 76
>
> 2 files changed, 85 insertions(+), 10 deletions(-)
>
> diff --git a/convert.c b/convert.c
> index 20d7ab67bd..63ef799239 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -220,18 +220,27 @@ static void check_safe_crlf(const char *path, enum
> crlf_action crlf_action,
> }
> }
>
> -static int has_cr_in_index(const struct index_state *istate, const char
> *path)
> +static int has_crlf_in_index(const struct index_state *istate, const char
> *path)
> {
> unsigned long sz;
> void *data;
> - int has_cr;
> + const char *crp;
> + int has_crlf = 0;
>
> data = read_blob_data_from_index(istate, path, &sz);
> if (!data)
> return 0;
> - has_cr = memchr(data, '\r', sz) != NULL;
> +
> + crp = memchr(data, '\r', sz);
> + if (crp && (crp[1] == '\n')) {
> + unsigned int ret_stats;
> + ret_stats = gather_convert_stats(data, sz);
> + if (!(ret_stats & CONVERT_STAT_BITS_BIN) &&
> + (ret_stats & CONVERT_STAT_BITS_TXT_CRLF))
> + has_crlf = 1;
> + }
> free(data);
> - return has_cr;
> + return has_crlf;
> }
>
> static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
> @@ -290,7 +299,7 @@ static int crlf_to_git(const struct index_state *istate,
>* cherry-pick.
>*/
> if ((checksafe != SAFE_CRLF_RENORMALIZE) &&
> - has_cr_in_index(istate, path))
> + has_crlf_in_index(istate, path))
> convert_crlf_into_lf = 0;
> }
> if ((checksafe == SAFE_CRLF_WARN ||
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index 68108d956a..0af35cfb1f 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -43,19 +43,31 @@ create_gitattributes () {
> } >.gitattributes
> }
>
> -create_NNO_files () {
> +# Create 2 sets of files:
> +# The NNO files are "Not NOrmalized in the repo. We use CRLF_mix_LF and store
> +# it under different names for the different test cases, see ${pfx}
> +# Depending on .gitattributes they are normalized at the next commit (or
> not)
> +# The MIX files have different contents in the repo.
> +# Depending on its contents, the "new safer autocrlf" may kick in.
> +create_NNO_MIX_files () {
> for crlf in false true input
> do
> for attr in "" auto text -text
> do
> for aeol in "" lf crlf
> do
> - pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
> + pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf} &&
> cp CRLF_mix_LF ${pfx}_LF.txt &&
> cp CRLF_mix_LF ${pfx}_CRLF.txt &&
> cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
> cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
> - cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
> + cp CRLF_mix_LF ${pfx}_CRLF_nul.txt &&
> + pfx=MIX_attr_${attr}_aeol_${aeol}_${crlf} &&
> + cp LF ${pfx}_LF.txt &&
> + cp CRLF${pfx}_CRLF.txt &&
> + cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
> + cp LF_mix_CR ${pfx}_LF_mix_CR.txt &&
> +
Re: [PATCH v2] gitcli: tweak "man gitcli" for clarity
Jeff King writes: > There is an open question of how carefully we want to document it, but I > think the strategy so far has been: > > - if you want to be careful, use "--" > > - if you don't, git will use black magic to guess, but that magic is >subject to change, so don't rely on it > > I don't mind documenting the current magic as long as the "don't rely on > it" part is made clear. Yes, taken with "git log master" example where if we want to say "this truly cannot be ambiguous" and end up digging "git log HEAD --" to ensure there is no path that match 'master' ever existed, I would prefer not to say a lot more about "black magic" and yet still going into the precise details. On the other hand, of course we do not want to cast in stone the precise details of the current "black magic" implementation that is subject to change. A description of "black magic" that is without details, i.e. the one that focuses on the spirit and not the exact design, would be... Without "--", Git tries to find a point between two arguments on (or at the beginning or the end of) the command line, where every argument before it are likely to be a revision (and unlikely to be a path) and every argument after it are likely to be a path (and unlikely to be a revision) with "black magic". If there is no such point, you'd be asked to disambiguate. The "black magic" would use 4 combinations that results from two tests. A. Is it likely to be a revision (yes/no)? B. Is it likely to be a path (yes/no)? If both are true, it is am ambigous command line. If neither is true, it is likely a typo (e.g. "git log naster" when the user meant 'master', or "git log Nakefile" when the user meant 'Makefile'). If only one is true, Git thinks that the command line is unambigous and goes ahead with its decision. Git will not spend excess amount of cycles to make these two tests, so there can be misidentification. Two easy to understand examples are: - If you have a file 'naster' in your working tree and said "git log naster", test A _could_ notice that there is a slightly different name 'master' that could be a revision that you meant, and ask you to disambiguate in case you made a typo. Because test A (deliberately) is not overly thorough, Git does not flag it as a possible ambiguity. - If you had a file whose name is "Nakefile" in HEAD but you just removed it, "git log Nakefile" may actually be a valid and unambigous request to use Nakefile as a path, but in order to notice that possibility, Git has to not just check if the working tree has such a path, but also in the index and HEAD (and if the removal was older, then it has to do an internal "git log" to see what paths ever existed in the past, which is ridiculous). Because test B (deliberately) is not overly thorough, Git would refuse to use it as either a revision or a path without disambiguation.
Re: RFC: Native clean/smudge filter for UTF-16 files
Jeff King writes: > So anyway, that is an alternate strategy, but I think I like "canonical > in-repo text is utf-8" approach a lot more, since then git operations > work consistently. There are still a few rough edges (e.g., I'm not sure Sounds like a good way forward. > if you could apply a utf-8 patch directly to a utf-16 working tree file. > Certainly not using "patch", but I'm not sure how well "git apply" would > handle that case either). But I think it would mostly Just Work as long > as people were willing to set their encoding attributes. It should work (or fail) just like applying LF patch to CRLF working tree, so I wouldn't worry too much about it. Thanks.
HELLO
-- Dear Friend, I am MR. WILFRED KABORE, THE MANAGER, BILL AND EXCHANGE DEPT., BANK OF AFRICA - Burkina Faso. I know this message will come to you as a surprise. Don't worry I was totally convinced to write you, I hope that you will not expose or betray this trust and confident that i am about to repose on you for the mutual benefits of our families. I need your urgent assistance in transferring the sum of US$12.8M(Twelve Million Eight Hundred Thousand USD) into your account within 10 to 14 working days. This requires a private arrangement; I shall forward to you the details of the transaction if you indicate your interest in this proposal. This money has been dormant for years in our bank without claim, I want the bank to release the money to you as the closest person to the late bank's customer (owner of the account) who died along with his entire family during the Iraq war in 2006. I don't want the money to go into our bank treasurer account as an abandoned fund. So this is the reason why i contact you so that the bank will release the money to you as the next of kin to the dead customer and we share the money together. Please i would like you to keep this proposal as a top secret. Upon receipt of your reply, I will give you full details on how the business deal will be executed and also note that you will have 40% of the above mentioned sum. Kindly fill out this information's requested below in return then i will give you more details with application form for the claim. Your full name: Your Country: Phone Number: Your Age : -- Your Occupation: -- I expect your urgent response if you agree to handle this project with me. Best Regards. Wilfred Kabore
Re: [PATCH] merge-recursive: ignore_case shouldn't reject intentional removals
On Fri, Nov 24, 2017 at 12:04 PM, Eric Sunshine wrote: > On Fri, Nov 24, 2017 at 2:59 PM, Elijah Newren wrote: >> In commit ae352c7f3 (merge-recursive.c: fix case-changing merge bug, >> 2014-05-01), it was observed that removing files could be problematic on >> case insensitive file systems, because we could end up removing files >> that differed in case only rather than deleting the intended file -- >> something that happened when files were renamed on one branch in a way >> that differed only in case. To avoid that problem, that commit added >> logic to avoid removing files other than the one intended, rejecting the >> removal if the files differed only in case. >> >> Unfortunately, the logic it used didn't fully implement that condition as >> stated above; instead it merely checked that a case-insensitive lookup of >> the file that was requested resulted in finding a file in the index at >> stage 0, not that the file found in the index actually differed in case. >> Alternatively, one could view the implementation as making an implicit >> assumption that the file we actually wanted to remove would never appear >> in the index with a stage of 0, and thus that if we found a file with our >> lookup, that it had to be a different file (but different in case only). >> >> The net result of this implementation is that it can ignore more requests >> than it should, leaving a file around in the working copy that should >> have been removed. Make sure that the file found in the index actually >> differs in case before silently ignoring the request to remove the file. >> >> --- > > Missing sign-off. Whoops! Signed-off-by: Elijah Newren
Re: [PATCH v3 00/33] Add directory rename detection to git
On Thu, Nov 23, 2017 at 9:25 PM, Elijah Newren wrote: > On Thu, Nov 23, 2017 at 2:28 PM, Elijah Newren wrote: >> On Thu, Nov 23, 2017 at 3:52 AM, Adam Dinwoodie wrote: >>> On Tuesday 21 November 2017 at 12:00 am -0800, Elijah Newren wrote: merge-recursive.c | 1243 +++- merge-recursive.h | 17 + t/t3501-revert-cherry-pick.sh |5 +- t/t6043-merge-rename-directories.sh | 3821 +++ t/t7607-merge-overwrite.sh |7 +- unpack-trees.c |4 +- unpack-trees.h |4 + 7 files changed, 4985 insertions(+), 116 deletions(-) create mode 100755 t/t6043-merge-rename-directories.sh >>> >>> The new t6043.44 introduced in this branch is failing on my Cygwin >>> system. I can't immeditely see what's causing the failure, but I've >>> copied the relevant verbose + shell tracing output below in the hope it >>> makes more sense to you: >> >> Thanks for reporting. Unfortunately, I have been unable to locate or >> create a cygwin system on which to replicate the testing. Valgrind is > > Nevermind; found a system that allowed me to reproduce the problem, > even if it far more wrangling than I'd like. I believe I have a > one-line fix, but I'm worried that attempting to fully explain it will > result in a novel-length commit message. This issue is addressed by the commit at https://public-inbox.org/git/20171124195901.2581-1-new...@gmail.com/ . I opted to make it a different series, because from my view it looks like a separate latent bug in the ignore_case handling that was just unearthed by a combination of events that included my series. It's conceivable it could have eventually been triggered some other way. However, that commit cleanly cherry picks to any of maint, master, next, or pu, so If Junio just wants to just include that commit on the top of the en/rename-directory-detection series, that's fine too. Without that commit, I get the same failure you did on cygwin. With that commit, I get all tests passing on cygwin for pu as of yesterday. (Well, all tests that ran; I didn't have svn or apache or p4 or various other things installed. Also, there were some unexpected passing TODOs, but I've already seen others discussing those exact testcases on the list elsewhere.) Hope that helps, Elijah
Re: [PATCH] merge-recursive: ignore_case shouldn't reject intentional removals
On Fri, Nov 24, 2017 at 2:59 PM, Elijah Newren wrote:
> In commit ae352c7f3 (merge-recursive.c: fix case-changing merge bug,
> 2014-05-01), it was observed that removing files could be problematic on
> case insensitive file systems, because we could end up removing files
> that differed in case only rather than deleting the intended file --
> something that happened when files were renamed on one branch in a way
> that differed only in case. To avoid that problem, that commit added
> logic to avoid removing files other than the one intended, rejecting the
> removal if the files differed only in case.
>
> Unfortunately, the logic it used didn't fully implement that condition as
> stated above; instead it merely checked that a case-insensitive lookup of
> the file that was requested resulted in finding a file in the index at
> stage 0, not that the file found in the index actually differed in case.
> Alternatively, one could view the implementation as making an implicit
> assumption that the file we actually wanted to remove would never appear
> in the index with a stage of 0, and thus that if we found a file with our
> lookup, that it had to be a different file (but different in case only).
>
> The net result of this implementation is that it can ignore more requests
> than it should, leaving a file around in the working copy that should
> have been removed. Make sure that the file found in the index actually
> differs in case before silently ignoring the request to remove the file.
>
> ---
Missing sign-off.
> diff --git a/merge-recursive.c b/merge-recursive.c
> index b48b15a6f..100fb913f 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -646,7 +646,7 @@ static int remove_file(struct merge_options *o, int clean,
> if (ignore_case) {
> struct cache_entry *ce;
> ce = cache_file_exists(path, strlen(path),
> ignore_case);
> - if (ce && ce_stage(ce) == 0)
> + if (ce && ce_stage(ce) == 0 && strcmp(path, ce->name))
> return 0;
> }
> if (remove_path(path))
> --
> 2.11.0
[PATCH] merge-recursive: ignore_case shouldn't reject intentional removals
In commit ae352c7f3 (merge-recursive.c: fix case-changing merge bug, 2014-05-01), it was observed that removing files could be problematic on case insensitive file systems, because we could end up removing files that differed in case only rather than deleting the intended file -- something that happened when files were renamed on one branch in a way that differed only in case. To avoid that problem, that commit added logic to avoid removing files other than the one intended, rejecting the removal if the files differed only in case. Unfortunately, the logic it used didn't fully implement that condition as stated above; instead it merely checked that a case-insensitive lookup of the file that was requested resulted in finding a file in the index at stage 0, not that the file found in the index actually differed in case. Alternatively, one could view the implementation as making an implicit assumption that the file we actually wanted to remove would never appear in the index with a stage of 0, and thus that if we found a file with our lookup, that it had to be a different file (but different in case only). The net result of this implementation is that it can ignore more requests than it should, leaving a file around in the working copy that should have been removed. Make sure that the file found in the index actually differs in case before silently ignoring the request to remove the file. --- If that description leaves more questions than answers, we may need to augment the above commit message with the following explanation... But, you may ask, why didn't we ever discover this problem before? And why would we have a file at stage 0 in the index that we wanted to remove from the working copy? Great questions, both, but the answer is fairly lengthy. The short answer to the first question is that due to a myriad of details, this bug is only currently triggerable: * on case insensitive filesystems * with rename/rename(2to1) conflicts * once one has taken care to avoid allowing renames to overwrite untracked or dirty files (see commit 30fd3a5 (merge overwrites unstaged changes in renamed file, 2012-04-15), which was fixed in my in-flight en/rename-directory-detection series currently sitting in pu) * AND where one of the renames is implicitly done (e.g. via directory rename detection). Thus, this bug remained hidden/latent until those other conditions are all triggered. Luckily, testcase 7b of t6043 added in en/rename-directory-detection was written to carefully check all details of the index and working copy to ensure they had the right content, giving us an early notification of this bug. (I was worried when I wrote those tests that I was being too laborious in checking all details, but I apparently got lucky and the extra checks in one of them paid off.) To explain why we would have a file at stage 0 in the index that we wanted to remove from the working copy can be explained by four points (most of which bring up further questions, but be patient and I'll try to wrap up all the loose ends): * If we have two conflicting files at PATH, we will want the index to have two higher stage entries for PATH. There are a few choices for the working copy, but one prominent choice is to remove PATH from the working copy, then create PATH~HEAD and PATH~$MERGE files. This strategy for the working copy is currently used for rename/rename(2to1) conflicts, though it has also been proposed (in my pending rename-perf series I submitted this month) for some add/add and rename/add conflicts. * When unpack_trees() (called by merge-recursive) notices two paths at the same location (which could mean an add/add conflict, or after rename detection it might more precisely turn out to be a rename/add or rename/rename(2to1) conflict), unpack_trees() removes the stage 0 index entry for the path and just creates higher stage entries. HOWEVER, if files can be implicitly renamed to a path that didn't exist on either side of the merge (such as from directory rename detection), then merge-recursive can see two conflicting files at the same location, despite unpack_trees() having left the path at stage 0. * merge-recursive is traditionally thought of as doing a 3-way merge. But what it really is forced to do is more of a 4-way merge; a good chunk of its annoying complexity is based around this (undocumented and unfortunate) reality. It derives from what I consider a simple design flaw. * In order to get the 4-way merge right (i.e. avoiding spurious error messages and taking care to not overwrite important dirty or untracked files), we MUST handle the working copy BEFORE updating the index. The combination of these four items in aggregate answers how we get a stage 0 file that we want to remove from the working copy, but leaves two questions -- what do I mean by 4-way merge, and
[PATCH] Fixed Russian translation
--- po/ru.po | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/po/ru.po b/po/ru.po index d4370b5941a13..4ce3d38047303 100644 --- a/po/ru.po +++ b/po/ru.po @@ -1,7 +1,7 @@ # SOME DESCRIPTIVE TITLE. # Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER # This file is distributed under the same license as the PACKAGE package. -# +# # Translators: # Dimitriy Ryazantcev , 2014-2017 # insolor, 2014 @@ -3421,7 +3421,7 @@ msgstr "обнаружены неизвестные расширения реп #: setup.c:806 #, c-format msgid "Not a git repository (or any of the parent directories): %s" -msgstr "Не найден git репозитоий (или один из его каталогов): %s" +msgstr "Не найден git репозиторий (или один из его каталогов): %s" #: setup.c:808 builtin/index-pack.c:1653 msgid "Cannot come back to cwd" @@ -3441,7 +3441,7 @@ msgstr "Не удалось изменить на «%s»" msgid "" "Not a git repository (or any parent up to mount point %s)\n" "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)." -msgstr "Не найден git репозитоий (или один из его каталогов вплоть до точки монтирования %s)\nОстанавливаю поиск на границе файловой системы (так как GIT_DISCOVERY_ACROSS_FILESYSTEM не установлен)." +msgstr "Не найден git репозиторий (или один из его каталогов вплоть до точки монтирования %s)\nОстанавливаю поиск на границе файловой системы (так как GIT_DISCOVERY_ACROSS_FILESYSTEM не установлен)." #: setup.c:1159 #, c-format @@ -7436,7 +7436,7 @@ msgstr "неправильный параметр: %s" #: builtin/diff.c:357 msgid "Not a git repository" -msgstr "Не найден git репозитоий" +msgstr "Не найден git репозиторий" #: builtin/diff.c:400 #, c-format -- https://github.com/git/git/pull/435
Re: [PATCH 1/1] convert: tighten the safe autocrlf handling
On Fri, Nov 24, 2017 at 12:24:48PM -0500, Eric Sunshine wrote:
> On Fri, Nov 24, 2017 at 11:14 AM, wrote:
> > When a text file had been commited with CRLF and the file is commited
> > again, the CRLF are kept if .gitattributs has "text=auto".
> > This is done by analyzing the content of the blob stored in the index:
> > If a '\r' is found, Git assumes that the blob was commited with CRLF.
> >
> > The simple search for a '\r' does not always work as expected:
> > A file is encoded in UTF-16 with CRLF and commited. Git treats it as binary.
> > Now the content is converted into UTF-8. At the next commit Git treats the
> > file as text, the CRLF should be converted into LF, but isn't.
> >
> > Solution:
> > Replace has_cr_in_index() with has_crlf_in_index(). When no '\r' is found,
> > 0 is returned directly, this is the most common case.
> > If a '\r' is found, the content is analyzed more deeply.
> >
> > Signed-off-by: Torsten Bögershausen
> > ---
> > diff --git a/convert.c b/convert.c
> > @@ -220,18 +220,27 @@ static void check_safe_crlf(const char *path, enum
> > crlf_action crlf_action,
> > -static int has_cr_in_index(const struct index_state *istate, const char
> > *path)
> > +static int has_crlf_in_index(const struct index_state *istate, const char
> > *path)
> > {
> > unsigned long sz;
> > void *data;
> > - int has_cr;
> > + const char *crp;
> > + int has_crlf = 0;
> >
> > data = read_blob_data_from_index(istate, path, &sz);
> > if (!data)
> > return 0;
> > - has_cr = memchr(data, '\r', sz) != NULL;
> > +
> > + crp = memchr(data, '\r', sz);
> > + if (crp && (crp[1] == '\n')) {
>
> If I understand correctly, this isn't a NUL-terminated string and it
> might be a binary blob, so if the lone CR in a file resides at the end
> of the file, won't this try looking for LF out-of-bounds? I would have
> expected the conditional to be:
>
> if (crp && crp - data + 1 < sz && crp[1] == '\n') {
>
> or any equivalent variation.
>
The read_blob_data_from_index() function should always append a '\0',
regardless if the blob is binary or not.
Re: [PATCH v2] gitcli: tweak "man gitcli" for clarity
On Fri, Nov 24, 2017 at 10:01:41AM +0900, Junio C Hamano wrote: > Actually the second example is a lot worse (and that is why I am > bringing it up). If git does spend cycles to realize that "git > could", for consistency, it must also check if "next" is unambiguous > between a path or a rev, i.e. it must dig history from "master" and > see if "next" appears as a path ever in the history, and if so, die > with "ambiguous argument". I just sent a similar response before reading this, and agree with everything you said. But I wanted to point out this "we must also look for ambiguities" argument, because I totally missed it in my response. And it's much more damning, I think, because it means you can never short-cut the easy cases and say "OK, we found the path, therefore we can stop our traversal early". To behave consistently, you have to always do the whole traversal twice. -Peff
Re: [PATCH v2] gitcli: tweak "man gitcli" for clarity
On Thu, Nov 23, 2017 at 09:55:03PM +0100, Kevin Daudt wrote: > > > > Without a disambiguating `--`, Git makes a reasonable guess. If it > > > > cannot guess (because your request is ambiguous), then it will error > > > > out. > [...] > > > 1) even without the "--", git can generally parse the command and do > > > the right thing (or do a *valid* thing, given its heuristics) > > > > > > 2) occasionally, without the "--", the command is really and truly > > > ambiguous, at which point git will fail and tell you to disambiguate > [...] > > Just for completeness, as it is somewhat covered by point 1 already, but > there are cases where there is no real ambiguity but you are required to > add '--' to tell git that it should not look for the file in the working > tree: Right, I was focused on what the sentence _currently_ said, and didn't think about other cases. The "cannot guess" case is not just due to ambiguity, but may be due to other heuristics. I _think_ the only one is the "does it exist in the working tree" rule you found, but I'm not sure we'd want to commit ourselves to never changing that. You could make my suggestion correct by putting "e.g.," or "for example" at the front of the parentheses. ;) There is an open question of how carefully we want to document it, but I think the strategy so far has been: - if you want to be careful, use "--" - if you don't, git will use black magic to guess, but that magic is subject to change, so don't rely on it I don't mind documenting the current magic as long as the "don't rely on it" part is made clear. > $ git show abc123 deleted_file.txt > fatal: ambiguous argument 'deleted_file.txt': > unknown revision or path not in the working tree. > Use '--' to separate paths from revisions, like this: > 'git [...] -- [...]' > > There might be good reasons why this is, but I don't consider this to be > actually ambiguous: there is no branch called 'deleted_file.txt' and git > could know that the files exists in the mentioned commit, so it should > be pretty clear what is meant. For that command, yes. But when the command is "git log", do we really want to dig through all of history to see if anybody ever mentions "deleted_file"? I'm not sure if we want to get into having different rules for different contexts. Not to mention that this really mixes up the layers; you cannot know what the whole command line means until you decide what abc123 means and examine it, which may in turn be influenced by other options. E.g., given: git log --no-merges A..B deleted_file.txt we have to actually do the no-merges log of A..B to see if deleted_file.txt is in there. -Peff
Re: [PATCH] bash completion: Add --autostash and --no-autostash to pull
> Ideally we should only autocomplete if pull has --rebase since > they only work with it but could not figure out how to do that > and the error message of doing git pull --autostash points out > that you need --rebase so i guess it's good enough You could use the completion script's __git_find_on_cmdline() helper function to easily check whether the '--rebase' option is already present on the command line. Having said that, I don't think we should go there, it feels that's trying to be overly and unnecessarily clever. After all, the order of command line options doesn't matter, and 'git pull --autostash --rebase' is a perfectly legit command. > Signed-off-by: Albert Astals Cid > --- > contrib/completion/git-completion.bash | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git- > completion.bash > index 539d7f84f..7ded58f38 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1923,6 +1923,7 @@ _git_pull () > --*) > __gitcomp " > --rebase --no-rebase > + --autostash --no-autostash > $__git_merge_options > $__git_fetch_options > " > -- > 2.15.0 > > >
Re: RFC: Native clean/smudge filter for UTF-16 files
On Thu, Nov 23, 2017 at 04:18:59PM +0100, Lars Schneider wrote: > Alternatively, I could add a native attribute to Git that translates UTF-16 > to UTF-8 and back. A conversion function is already available in "mingw.h" [3] > on Windows. Limiting this feature to Windows wouldn't be a problem from my > point of view as UTF-16 is only relevant on Windows anyways. The attribute > could look like this: > > *.txttext encoding=utf-16 > > There was a previous discussion on the topic and Jonathan already suggested > a "native" clean/smudge filter in 2010 [4]. Also the "encoding" attribute > is already present but, as far as I can tell, is only used by the git gui > for viewing [5]. I would not want to see a proliferation of built-in filters, but it really seems like text-encoding conversion is a broad and practical one that many people might benefit from. So just like line-ending conversion, which _could_ be done by generic filters, it makes sense to me to support it natively for speed and simplicity. > Do you think a patch that converts UTF-16 files to UTF-8 via an attribute > "encoding=utf-16" on Windows would have a chance to get accepted? You haven't fully specified the semantics here, so let me sketch out what I think it ought to look like: - declare utf8 the "canonical" in-repo representation, just as we have declared LF for line endings (alternatively this could be configurable, but if we can get away with declaring utf8 the one true encoding, that cuts out a lot of corner cases). - if core.convertEncoding is true, then for any file with an encoding=foo attribute, internally run iconv(foo, utf8) in convert_to_git(), and likewise iconv(utf8, foo) in convert_to_working_tree. - I'm not sure if core.convertEncoding should be enabled by default. If it's a noop as long as there's no encoding attribute, then it's probably fine. But I would not want accidental conversion or any slowdown for the common case that the user wants no conversion. - I doubt we'd want a "core.autoEncoding" similar to "core.autocrlf". I don't think people consistently have all utf-16 files (the way they might have all CRLF files) rather a few files that must be utf-16. - I have actually seen two types of utf-16 in git repos in the wild: files which really must be utf-16 (because some tool demands it) and files which happen to be utf-16, but could just as easily be utf-8 (and the user simply does not notice and commits utf-16, but doesn't realize it until much later when their diffs are unreadable). For the first case, the "encoding" thing above would work fine. For the second case, in theory we could have an option that takes any file with a "text" attribute and no "encoding" attribute, and converts it to utf-8. I suspect that's opening a can of worms for false positives similar to core.autocrlf. And performance drops as we try to guess the encoding and convert all incoming data. So I mention it mostly as a direction I think we probably _don't_ want to go. Anybody with the "this could have been utf-8 all along" type of file can remedy it by converting and committing the result. Omitting all of the "we shouldn't do this" bullet points, it seems pretty simple and sane to me. There is one other approach, which is to really store utf-16 in the repository and better teach the diff tools to handle it (which are really the main thing in git that cares about looking into the blob contents). You can do this already with a textconv filter, but: 1. It's slow (though cacheable). 2. It doesn't work unless each repo configures the filter (so not on sites like GitHub, unless we define a micro-format that diff=utf16 should be textconv'd on display, and get all implementations to respect that). 3. Textconv patches look good, but can't be applied. This occasionally makes things awkward, depending on your workflow. 4. You have to actually mark each file with an attribute, which is slightly annoying and more thing to remember to do (I see this from the server side, since people often commit utf-16 without any attributes, and then get annoyed when they see the file marked as binary). We've toyed with the idea at GitHub of auto-detecting UTF-16 BOMs and doing an "auto-textconv" to utf-8 (for our human-readable diffs only, of course). That solves (1), (2), and (4), but leaves (3). I actually looked into using libicu to do it not just for UTF-16, but to detect any encoding. It turned out to be really slow, though. :) So anyway, that is an alternate strategy, but I think I like "canonical in-repo text is utf-8" approach a lot more, since then git operations work consistently. There are still a few rough edges (e.g., I'm not sure if you could apply a utf-8 patch directly to a utf-16 working tree file. Certainly not using "patch", but I'm not sure how well "git apply" would handle that case either). But I th
RE
Hello Dear I am Mr.Sheng Li Hung, from china I got your information while search for a reliable person, I have a very profitable business proposition for you and i can assure you that you will not regret been part of this mutual beneficial transaction after completion. Kindly get back to me for more details on this email id: shenglil...@hotmail.com Thanks Sheng Li Hung
Re: [PATCH 1/1] convert: tighten the safe autocrlf handling
On Fri, Nov 24, 2017 at 11:14 AM, wrote:
> When a text file had been commited with CRLF and the file is commited
> again, the CRLF are kept if .gitattributs has "text=auto".
> This is done by analyzing the content of the blob stored in the index:
> If a '\r' is found, Git assumes that the blob was commited with CRLF.
>
> The simple search for a '\r' does not always work as expected:
> A file is encoded in UTF-16 with CRLF and commited. Git treats it as binary.
> Now the content is converted into UTF-8. At the next commit Git treats the
> file as text, the CRLF should be converted into LF, but isn't.
>
> Solution:
> Replace has_cr_in_index() with has_crlf_in_index(). When no '\r' is found,
> 0 is returned directly, this is the most common case.
> If a '\r' is found, the content is analyzed more deeply.
>
> Signed-off-by: Torsten Bögershausen
> ---
> diff --git a/convert.c b/convert.c
> @@ -220,18 +220,27 @@ static void check_safe_crlf(const char *path, enum
> crlf_action crlf_action,
> -static int has_cr_in_index(const struct index_state *istate, const char
> *path)
> +static int has_crlf_in_index(const struct index_state *istate, const char
> *path)
> {
> unsigned long sz;
> void *data;
> - int has_cr;
> + const char *crp;
> + int has_crlf = 0;
>
> data = read_blob_data_from_index(istate, path, &sz);
> if (!data)
> return 0;
> - has_cr = memchr(data, '\r', sz) != NULL;
> +
> + crp = memchr(data, '\r', sz);
> + if (crp && (crp[1] == '\n')) {
If I understand correctly, this isn't a NUL-terminated string and it
might be a binary blob, so if the lone CR in a file resides at the end
of the file, won't this try looking for LF out-of-bounds? I would have
expected the conditional to be:
if (crp && crp - data + 1 < sz && crp[1] == '\n') {
or any equivalent variation.
> + unsigned int ret_stats;
> + ret_stats = gather_convert_stats(data, sz);
> + if (!(ret_stats & CONVERT_STAT_BITS_BIN) &&
> + (ret_stats & CONVERT_STAT_BITS_TXT_CRLF))
> + has_crlf = 1;
> + }
> free(data);
> - return has_cr;
> + return has_crlf;
> }
[PATCH] checkout: include the checkout.h header file
Signed-off-by: Ramsay Jones
---
Hi Thomas,
If you need to re-roll your 'tg/worktree-create-tracking' branch, could
you please squash this into the relevant patch (commit 6736ae9593,
"checkout: factor out functions to new lib file", 2017-11-22).
[noticed by sparse]
Thanks!
ATB,
Ramsay Jones
checkout.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/checkout.c b/checkout.c
index b0c744d37..ac42630f7 100644
--- a/checkout.c
+++ b/checkout.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "remote.h"
+#include "checkout.h"
struct tracking_name_data {
/* const */ char *src_ref;
--
2.15.0
Re: [PATCH 5/5] sha1_file: don't re-scan pack directory for null sha1
On Thu, Nov 23, 2017 at 11:35:21AM +0900, Junio C Hamano wrote:
> Not limiting us to the caller in the "fetch" codepath, I think the
> expectation by callers of lookup_commit_reference_gently() in the
> ideal world would be:
>
> - It has an object name, and wants to use it as point in the commit
>DAG to define the traversal over the DAG, if it refers to a
>commit known to us.
>
> - It does not know if these object names represent a tag object, a
>commit object, or some other object. It does not know if the
>local repository actually has them (e.g. we received a "have"
>from the other side---missing is expected).
>
> - Hence, it would happily accept a NULL as "we do not have it" and
>"we do have it, but it is not a commit-ish".
>
> And from that point of view, 2, 3a (missing), and 4 (0{40}) to yield
> NULL is perfectly fine. 3b (exists but broken) may be a noteworthy
> event, but for the purpose of the caller, it may want to proceed as
> if the object is missing from our end, so it might deserve warning()
> but not die(), at least as the default behaviour.
Hmm, yeah, I did not differentiate 3a and 3b in my analysis. How we'd
want to handle "missing" would vary from caller to caller, I think.
Certainly for this case in "fetch" where it's the "old" value for a ref,
it would be a corruption not to have it. Just grepping around a few of
the other callers, I see:
- archive.c:parse_treeish_arg: fine not to have it (we'd complain soon
after that it doesn't point to a tree either). But also fine to
complain hard.
- blame.c:dwim_reverse_initial, and checkout_paths and switch_branches
in checkout.c: missing object would mean a broken HEAD
- show-branch.c:append_ref: missing would mean a broken ref
- clear_commit_marks_for_object_array: conceptually OK to have a
missing object, though I suspect practically impossible since we
examined and marked the objects earlier
- ref-filter's --merged, --contains, etc: the low-level code quietly
ignores a missing object or non-commit, but the command-line parser
enforces that we find a commit. So probably impossible to trigger,
but arguably the second call should be a BUG().
So I dunno. That is far from exhaustive, but it does seem like most
cases should assume the presence of the file.
As for your proposed behavior:
> And from that point of view, 2, 3a (missing), and 4 (0{40}) to yield
> NULL is perfectly fine. 3b (exists but broken) may be a noteworthy
> event, but for the purpose of the caller, it may want to proceed as
> if the object is missing from our end, so it might deserve warning()
> but not die(), at least as the default behaviour.
That's actually not far from what happens now, with the only difference
being that we _do_ actually die() on a corruption (really any error
except ENOENT). I forgot that when I wrote my earlier message. You can
see it by updating the "fetch" reproduction I sent earlier to corrupt:
-- >8 --
git init parent
git -C parent commit --allow-empty -m base
git clone parent child
git -C parent commit --allow-empty -m more
cd child
for i in .git/objects/??/*
do
chmod +w $i
echo corrupt >$i
done
git fetch
-- 8< --
which gives something like:
error: inflate: data stream error (incorrect header check)
error: unable to unpack 55c66a401fd4190382f9cd8b70c11f9f5adb044e header
fatal: loose object 55c66a401fd4190382f9cd8b70c11f9f5adb044e (stored in
.git/objects/55/c66a401fd4190382f9cd8b70c11f9f5adb044e) is corrupt
So that's good. I do still think that many of the callers of
lookup_commit_reference_gently() probably ought to die() in the
"missing" case rather than continue (and produce subtly wrong answers).
But it may not be that big a deal. For the most part, all bets are off
in a corrupt repo. My main concern is just that we do not want the
corruption to spread or to make it harder for us to recover from (and
that includes allowing us to delete or overwrite other data that would
otherwise be forbidden, which is what's happening in the fetch case).
Most of the callers probably don't fall into that situation, but it
might be nice to err on the side of caution.
-Peff
[PATCH 1/1] convert: tighten the safe autocrlf handling
From: Torsten Bögershausen
When a text file had been commited with CRLF and the file is commited
again, the CRLF are kept if .gitattributs has "text=auto".
This is done by analyzing the content of the blob stored in the index:
If a '\r' is found, Git assumes that the blob was commited with CRLF.
The simple search for a '\r' does not always work as expected:
A file is encoded in UTF-16 with CRLF and commited. Git treats it as binary.
Now the content is converted into UTF-8. At the next commit Git treats the
file as text, the CRLF should be converted into LF, but isn't.
Solution:
Replace has_cr_in_index() with has_crlf_in_index(). When no '\r' is found,
0 is returned directly, this is the most common case.
If a '\r' is found, the content is analyzed more deeply.
Reported-By: Ashish Negi
Signed-off-by: Torsten Bögershausen
---
convert.c| 19 +
t/t0027-auto-crlf.sh | 76
2 files changed, 85 insertions(+), 10 deletions(-)
diff --git a/convert.c b/convert.c
index 20d7ab67bd..63ef799239 100644
--- a/convert.c
+++ b/convert.c
@@ -220,18 +220,27 @@ static void check_safe_crlf(const char *path, enum
crlf_action crlf_action,
}
}
-static int has_cr_in_index(const struct index_state *istate, const char *path)
+static int has_crlf_in_index(const struct index_state *istate, const char
*path)
{
unsigned long sz;
void *data;
- int has_cr;
+ const char *crp;
+ int has_crlf = 0;
data = read_blob_data_from_index(istate, path, &sz);
if (!data)
return 0;
- has_cr = memchr(data, '\r', sz) != NULL;
+
+ crp = memchr(data, '\r', sz);
+ if (crp && (crp[1] == '\n')) {
+ unsigned int ret_stats;
+ ret_stats = gather_convert_stats(data, sz);
+ if (!(ret_stats & CONVERT_STAT_BITS_BIN) &&
+ (ret_stats & CONVERT_STAT_BITS_TXT_CRLF))
+ has_crlf = 1;
+ }
free(data);
- return has_cr;
+ return has_crlf;
}
static int will_convert_lf_to_crlf(size_t len, struct text_stat *stats,
@@ -290,7 +299,7 @@ static int crlf_to_git(const struct index_state *istate,
* cherry-pick.
*/
if ((checksafe != SAFE_CRLF_RENORMALIZE) &&
- has_cr_in_index(istate, path))
+ has_crlf_in_index(istate, path))
convert_crlf_into_lf = 0;
}
if ((checksafe == SAFE_CRLF_WARN ||
diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 68108d956a..0af35cfb1f 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -43,19 +43,31 @@ create_gitattributes () {
} >.gitattributes
}
-create_NNO_files () {
+# Create 2 sets of files:
+# The NNO files are "Not NOrmalized in the repo. We use CRLF_mix_LF and store
+# it under different names for the different test cases, see ${pfx}
+# Depending on .gitattributes they are normalized at the next commit (or not)
+# The MIX files have different contents in the repo.
+# Depending on its contents, the "new safer autocrlf" may kick in.
+create_NNO_MIX_files () {
for crlf in false true input
do
for attr in "" auto text -text
do
for aeol in "" lf crlf
do
- pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf}
+ pfx=NNO_attr_${attr}_aeol_${aeol}_${crlf} &&
cp CRLF_mix_LF ${pfx}_LF.txt &&
cp CRLF_mix_LF ${pfx}_CRLF.txt &&
cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
cp CRLF_mix_LF ${pfx}_LF_mix_CR.txt &&
- cp CRLF_mix_LF ${pfx}_CRLF_nul.txt
+ cp CRLF_mix_LF ${pfx}_CRLF_nul.txt &&
+ pfx=MIX_attr_${attr}_aeol_${aeol}_${crlf} &&
+ cp LF ${pfx}_LF.txt &&
+ cp CRLF${pfx}_CRLF.txt &&
+ cp CRLF_mix_LF ${pfx}_CRLF_mix_LF.txt &&
+ cp LF_mix_CR ${pfx}_LF_mix_CR.txt &&
+ cp CRLF_nul${pfx}_CRLF_nul.txt
done
done
done
@@ -136,6 +148,49 @@ commit_chk_wrnNNO () {
'
}
+# Commit a file with mixed line endings on top of different files
+# in the index. Check for warnings
+commit_MIX_chkwrn () {
+ attr=$1 ; shift
+ aeol=$1 ; shift
+ crlf=$1 ; shift
+ lfwarn=$1 ; shift
+ crlfwarn=$1 ; shift
+ lfmixcrlf=$1 ; shift
+ lfmixcr=$1 ; shift
+ crlfnul=$1 ; shift
+ pfx=MIX_attr_${attr}_aeol_${aeol}_${crlf}
+ #Commit file with CLRF_mix_LF on top of existing file
+ create_gitattributes "$attr" $a
Re: [PATCH v4 7/9] sequencer: load commit related config
On 24/11/17 13:48, Junio C Hamano wrote: > Phillip Wood writes: > >> From: Phillip Wood >> >> Load default values for message cleanup and gpg signing of commits in >> preparation for committing without forking 'git commit'. Note that we >> interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE to >> be consistent with 'git commit' > > Hmph, is that because we never invoke the editor to edit the commit > log message? Over there, scissors is demoted to space when the > editor is not in use, but otherwise this demotion does not occur. Yes that's right. In fact I'm fairly we always specify an explicit cleanup mode when calling try_to_commit() so the default is there in case that changes in the future. > Just being curious. > >> >> Signed-off-by: Phillip Wood >> --- >> >> Notes: >> changes since v3: >> - interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE >>to match 'git commit'
RE: Re: Unify annotated and non-annotated tags
On November 24, 2017 4:52 AM anatoly techtonik wrote: >On Thu, Nov 23, 2017 at 6:08 PM, Randall S. Becker >wrote: >> On 2017-11-23 02:31 (GMT-05:00) anatoly techtonik wrote >>>Subject: Re: Unify annotated and non-annotated tags On Sat, Nov 11, >>>2017 at 5:06 AM, Junio C Hamano wrote: Igor Djordjevic writes: > If you would like to mimic output of "git show-ref", repeating > commits for each tag pointing to it and showing full tag name as > well, you could do something like this, for example: > > for tag in $(git for-each-ref --format="%(refname)" refs/tags) > do > printf '%s %s\n' "$(git rev-parse $tag^0)" "$tag" > done > > > Hope that helps a bit. If you use for-each-ref's --format option, you could do something like (pardon a long line): git for-each-ref --format='%(if)%(*objectname)%(then)%(*objectname)%(else)%(objectnam e)%(end) %(refname)' refs/tags without any loop, I would think. >>>Thanks. That helps. >>>So my proposal is to get rid of non-annotated tags, so to get all tags >>>with commits that they point to, one would use: >>>git for-each-ref --format='%(*objectname) %(refname)' refs/tags> For >>>so-called non-annotated tags just leave the message empty. >>>I don't see why anyone would need non-annotated tags though. >> >> I have seen non-annotated tags used in automations (not necessarily well >> written ones) that >> create tags as a record of automation activity. I am not sure we should be >> writing off the >> concept of unannotated tags entirely. This may cause breakage based on >> existing expectations >> of how tags work at present. My take is that tags should include whodunnit, >> even if it's just the >> version of the automation being used, but I don't always get to have my >> wishes fulfilled. In >> essence, whatever behaviour a non-annotated tag has now may need to be >> emulated in >> future even if reconciliation happens. An option to preserve empty tag >> compatibility with >> pre-2.16 behaviour, perhaps? Sadly, I cannot supply examples of this usage >> based on a >> human memory page-fault and NDAs. >Are there any windows for backward compatibility breaks, or git is doomed to >preserve it forever? >Automation without support won't survive for long, and people who rely on that, >like Chromium team, usually hard set the version used. Just pointing out that changing the semantics of a basic data item in git may have unintended consequences.
Re: [PATCH v2] doc: clarify that "git bisect" accepts one or more good commits
Junio C Hamano writes: > "Robert P. J. Day" writes: > >> in this sense, i don't think "indicate" and "identify" are >> completely interchangeable. in my mind, the word "identify" does >> nothing more than, you know, point at something and say, "that one, >> that's the one i'm talking about;" it goes no further than that. >> >> on the other hand, the word "indicate" (in my mind) implies that >> you're about to provide some *property* or *quality* of something, and >> you do exactly that in the earlier quote: > > I do not think the two words have different connotations, so we are > in agreement. You do not necessarily need a property in mind when Eek. I do do think the two words are not interchangeable. I should stop typing late at night.
What's cooking in git.git (Nov 2017, #07; Fri, 24)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. Quite a few "fixes" have been accumulated on 'master' and already merged to 'maint' in preparation for 2.15.1; perhaps we can tag it near the end of the month--no fix in the mix is an ultra urgent one. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * en/rename-directory-detection (2017-11-21) 33 commits - merge-recursive: ensure we write updates for directory-renamed file - merge-recursive: avoid spurious rename/rename conflict from dir renames - directory rename detection: new testcases showcasing a pair of bugs - merge-recursive: fix remaining directory rename + dirty overwrite cases - merge-recursive: fix overwriting dirty files involved in renames - merge-recursive: avoid clobbering untracked files with directory renames - merge-recursive: apply necessary modifications for directory renames - merge-recursive: when comparing files, don't include trees - merge-recursive: check for file level conflicts then get new name - merge-recursive: add computation of collisions due to dir rename & merging - merge-recursive: add a new hashmap for storing file collisions - merge-recursive: check for directory level conflicts - merge-recursive: add get_directory_renames() - merge-recursive: add a new hashmap for storing directory renames - merge-recursive: split out code for determining diff_filepairs - merge-recursive: make !o->detect_rename codepath more obvious - merge-recursive: fix leaks of allocated renames and diff_filepairs - merge-recursive: introduce new functions to handle rename logic - merge-recursive: move the get_renames() function - directory rename detection: tests for handling overwriting dirty files - directory rename detection: tests for handling overwriting untracked files - directory rename detection: miscellaneous testcases to complete coverage - directory rename detection: testcases exploring possibly suboptimal merges - directory rename detection: more involved edge/corner testcases - directory rename detection: testcases checking which side did the rename - directory rename detection: files/directories in the way of some renames - directory rename detection: partially renamed directory testcase/discussion - directory rename detection: testcases to avoid taking detection too far - directory rename detection: directory splitting testcases - directory rename detection: basic testcases - merge-recursive: add explanation for src_entry and dst_entry - merge-recursive: fix logic ordering issue - Tighten and correct a few testcases for merging and cherry-picking Rename detection logic in "diff" family that is used in "merge" has learned to guess when all of x/a, x/b and x/c have moved to z/a, z/b and z/c, it is likely that x/d added in the meantime would also want to move to z/d by taking the hint that the entire directory 'x' moved to 'z'. Needs review. * ac/complete-pull-autostash (2017-11-22) 1 commit - completion: add --autostash and --no-autostash to pull The shell completion (in contrib/) learned that "git pull" can take the "--autostash" option. Will merge to 'next'. * cc/git-packet-pm (2017-11-22) 2 commits - Git/Packet.pm: use 'if' instead of 'unless' - Git/Packet: clarify that packet_required_key_val_read allows EOF Code clean-up. Will merge to 'next'. * jn/reproducible-build (2017-11-22) 3 commits - Merge branch 'jn/reproducible-build' of ../git-gui into jn/reproducible-build - git-gui: sort entries in optimized tclIndex - generate-cmdlist: avoid non-deterministic output The build procedure has been taught to avoid some unnecessary instability in the build products. Will merge to 'next'. * jt/submodule-tests-cleanup (2017-11-22) 1 commit - Tests: clean up submodule recursive helpers Further test clean-up. Will merge to 'next'. * rd/man-prune-progress (2017-11-22) 1 commit - prune: add "--progress" to man page and usage msg Doc update. Will merge to 'next'. * rd/man-reflog-add-n (2017-11-22) 1 commit - doc: add missing "-n" (dry-run) option to reflog man page Doc update. Will merge to 'next'. * ph/stash-save-m-option-fix (2017-11-24) 1 commit - stash: learn to parse -m/--message like commit does In addition to "git stash -m message", the command learned to accept "git stash -mmessage" form. Will merge to 'next'. * ra/decorate-limit-refs (2017-11-22) 1 commit - log: add option to choose which refs to decorate The tagnames "git log --decorate" uses to annotate the commits can now be limited to subset of available refs with the two additional options,
RE: offering
I have been trying to reach you contact me--(jonathansymonds...@hotmail.com) for more details
Re: [PATCH v4 7/9] sequencer: load commit related config
Phillip Wood writes: > From: Phillip Wood > > Load default values for message cleanup and gpg signing of commits in > preparation for committing without forking 'git commit'. Note that we > interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE to > be consistent with 'git commit' Hmph, is that because we never invoke the editor to edit the commit log message? Over there, scissors is demoted to space when the editor is not in use, but otherwise this demotion does not occur. Just being curious. > > Signed-off-by: Phillip Wood > --- > > Notes: > changes since v3: > - interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE >to match 'git commit'
Re: [PATCH v2] doc: clarify that "git bisect" accepts one or more good commits
"Robert P. J. Day" writes: > in this sense, i don't think "indicate" and "identify" are > completely interchangeable. in my mind, the word "identify" does > nothing more than, you know, point at something and say, "that one, > that's the one i'm talking about;" it goes no further than that. > > on the other hand, the word "indicate" (in my mind) implies that > you're about to provide some *property* or *quality* of something, and > you do exactly that in the earlier quote: I do not think the two words have different connotations, so we are in agreement. You do not necessarily need a property in mind when you "identify" a commit. You could just "identify" this and that commits to yourself, to keep them in mind. You _also_ can have a property in mind and "identify" this commit as a good one, and that commit as a bad one. But identifying a commit as bad (or another one as good) alone to yourself does not get anything started. In order to advance the bisection process, you need to tell the "git bisect" machinery that "this commit is good", "that commit is bad", etc. I would think "indicate" is a verb with better connotation than "identify" for describing that action. You indicate something *to* *somebody, and in this case you indicate that this commit is good and that commit is bad to git.
Urgent Message
Dear Western Union Customer, You have been awarded with the sum of $250,000 USD by our office, as one of our customers who use Western Union in their daily business transaction. This award was been selected through the internet, where your e-mail address was indicated and notified. Please provide Mr. James Udo with the following details listed below so that your fund will be remited to you through Western Union. 1. Name: 2. Address: 3. Country: 4. Phone Number: 5. Occupation: 6. Sex: 7. Age: Mr. James Udo E-mail: westerrnunion2...@outlook.com As soon as these details are received and verified, your fund will be transferred to you.Thank you, for using western union.
[PATCH v4 5/9] commit: move print_commit_summary() to libgit
From: Phillip Wood
Move print_commit_summary() from builtin/commit.c to sequencer.c so it
can be shared with other commands. The function is modified by
changing the last argument to a flag so callers can specify whether
they want to show the author date in addition to specifying if this is
an initial commit.
If the sequencer dies in print_commit_summary() (which can only happen
when cherry-picking or reverting) then neither the todo list nor the
abort safety file are updated to reflect the commit that was just
made. print_commit_summary() can die if:
- The commit that was just created cannot be found or parsed.
- HEAD cannot be resolved either because some other process is
updating it (which is bad news in the middle of a cherry-pick) or
because it is corrupt.
- log_tree_commit() cannot read some objects.
In all those cases dying will leave the sequencer in a sane state for
aborting; 'git cherry-pick --abort' will rewind HEAD to the last
successful commit before there was a problem with HEAD or the object
database. If the user somehow fixes the problem and runs 'git
cherry-pick --continue' then the sequencer will try and pick the same
commit again which may or may not be what the user wants depending on
what caused print_commit_summary() to die. If print_commit_summary()
returned an error instead then update_abort_safety_file() would try to
resolve HEAD which may or may not be successful. If it is successful
then running 'git rebase --abort' would not rewind HEAD to the last
successful commit which is not what we want.
Signed-off-by: Phillip Wood
---
Notes:
changes since v2:
- expanded commit message to explain why it is ok to die in
print_commit_summary() and dropped the next patch which made it
return an error instead.
- style fixes.
changes since v1:
- convert flags passed to print_commit_summary() to unsigned int
builtin/commit.c | 128 ---
sequencer.c | 119 +++
sequencer.h | 5 +++
3 files changed, 133 insertions(+), 119 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index
d251cfcebad3476c365492d83803e7821fdfdf2b..2043479d37873671d43124dc0cb509d6d9247baa
100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -43,31 +43,6 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static const char implicit_ident_advice_noconfig[] =
-N_("Your name and email address were configured automatically based\n"
-"on your username and hostname. Please check that they are accurate.\n"
-"You can suppress this message by setting them explicitly. Run the\n"
-"following command and follow the instructions in your editor to edit\n"
-"your configuration file:\n"
-"\n"
-"git config --global --edit\n"
-"\n"
-"After doing this, you may fix the identity used for this commit with:\n"
-"\n"
-"git commit --amend --reset-author\n");
-
-static const char implicit_ident_advice_config[] =
-N_("Your name and email address were configured automatically based\n"
-"on your username and hostname. Please check that they are accurate.\n"
-"You can suppress this message by setting them explicitly:\n"
-"\n"
-"git config --global user.name \"Your Name\"\n"
-"git config --global user.email y...@example.com\n"
-"\n"
-"After doing this, you may fix the identity used for this commit with:\n"
-"\n"
-"git commit --amend --reset-author\n");
-
static const char empty_amend_advice[] =
N_("You asked to amend the most recent commit, but doing so would make\n"
"it empty. You can repeat your command with --allow-empty, or you can\n"
@@ -1374,98 +1349,6 @@ int cmd_status(int argc, const char **argv, const char
*prefix)
return 0;
}
-static const char *implicit_ident_advice(void)
-{
- char *user_config = expand_user_path("~/.gitconfig", 0);
- char *xdg_config = xdg_config_home("config");
- int config_exists = file_exists(user_config) || file_exists(xdg_config);
-
- free(user_config);
- free(xdg_config);
-
- if (config_exists)
- return _(implicit_ident_advice_config);
- else
- return _(implicit_ident_advice_noconfig);
-
-}
-
-static void print_summary(const char *prefix, const struct object_id *oid,
- int initial_commit)
-{
- struct rev_info rev;
- struct commit *commit;
- struct strbuf format = STRBUF_INIT;
- const char *head;
- struct pretty_print_context pctx = {0};
- struct strbuf author_ident = STRBUF_INIT;
- struct strbuf committer_ident = STRBUF_INIT;
-
- commit = lookup_commit(oid);
- if (!commit)
- die(_("couldn't look up newly created commit"));
- if (parse_commit(commit))
- die(_("could not parse newly created commit"));
-
- strbuf_addstr(&format, "format:%h] %s");
-
- format_commit_message(commit, "%an <%ae>",
[PATCH v4 3/9] Add a function to update HEAD after creating a commit
From: Phillip Wood
Add update_head_with_reflog() based on the code that updates HEAD
after committing in builtin/commit.c that can be called by 'git
commit' and other commands.
Signed-off-by: Phillip Wood
---
Notes:
changes since v2:
- updated commit message to reflect the change in function name
- style fixes
changes since v1:
- rename update_head() to update_head_with_reflog()
builtin/commit.c | 20 ++--
sequencer.c | 39 ++-
sequencer.h | 4
3 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index
d958c2eb2adc9a29dab29340ce9b56daea41fecd..eb144556bf37b7bf357bd976b94305171b4fd159
100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1610,13 +1610,11 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
struct strbuf sb = STRBUF_INIT;
struct strbuf author_ident = STRBUF_INIT;
const char *index_file, *reflog_msg;
- char *nl;
struct object_id oid;
struct commit_list *parents = NULL;
struct stat statbuf;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
- struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -1739,25 +1737,11 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
strbuf_release(&author_ident);
free_commit_extra_headers(extra);
- nl = strchr(sb.buf, '\n');
- if (nl)
- strbuf_setlen(&sb, nl + 1 - sb.buf);
- else
- strbuf_addch(&sb, '\n');
- strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
- strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);
-
- transaction = ref_transaction_begin(&err);
- if (!transaction ||
- ref_transaction_update(transaction, "HEAD", &oid,
- current_head
- ? ¤t_head->object.oid : &null_oid,
- 0, sb.buf, &err) ||
- ref_transaction_commit(transaction, &err)) {
+ if (update_head_with_reflog(current_head, &oid, reflog_msg, &sb,
+ &err)) {
rollback_index_files();
die("%s", err.buf);
}
- ref_transaction_free(transaction);
unlink(git_path_cherry_pick_head());
unlink(git_path_revert_head());
diff --git a/sequencer.c b/sequencer.c
index
36e03d041f32bcc0fdd1fddebb33b23c7e4d8a70..ef262980c5255d90ee023c0b29c6c1c628b3c7d2
100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1,10 +1,10 @@
#include "cache.h"
#include "config.h"
#include "lockfile.h"
-#include "sequencer.h"
#include "dir.h"
#include "object.h"
#include "commit.h"
+#include "sequencer.h"
#include "tag.h"
#include "run-command.h"
#include "exec_cmd.h"
@@ -752,6 +752,43 @@ int template_untouched(const struct strbuf *sb, const char
*template_file,
return rest_is_empty(sb, start - sb->buf);
}
+int update_head_with_reflog(const struct commit *old_head,
+ const struct object_id *new_head,
+ const char *action, const struct strbuf *msg,
+ struct strbuf *err)
+{
+ struct ref_transaction *transaction;
+ struct strbuf sb = STRBUF_INIT;
+ const char *nl;
+ int ret = 0;
+
+ if (action) {
+ strbuf_addstr(&sb, action);
+ strbuf_addstr(&sb, ": ");
+ }
+
+ nl = strchr(msg->buf, '\n');
+ if (nl) {
+ strbuf_add(&sb, msg->buf, nl + 1 - msg->buf);
+ } else {
+ strbuf_addbuf(&sb, msg);
+ strbuf_addch(&sb, '\n');
+ }
+
+ transaction = ref_transaction_begin(err);
+ if (!transaction ||
+ ref_transaction_update(transaction, "HEAD", new_head,
+ old_head ? &old_head->object.oid : &null_oid,
+ 0, sb.buf, err) ||
+ ref_transaction_commit(transaction, err)) {
+ ret = -1;
+ }
+ ref_transaction_free(transaction);
+ strbuf_release(&sb);
+
+ return ret;
+}
+
static int is_original_commit_empty(struct commit *commit)
{
const struct object_id *ptree_oid;
diff --git a/sequencer.h b/sequencer.h
index
82e57713a2940c5d65ccac013c3f42c55cc12baf..81a2098e900f0aca30e45ed7f19ae4bf3ce682f0
100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -69,4 +69,8 @@ int message_is_empty(const struct strbuf *sb,
enum commit_msg_cleanup_mode cleanup_mode);
int template_untouched(const struct strbuf *sb, const char *template_file,
enum commit_msg_cleanup_mode cleanup_mode);
+int update_head_with_reflog(const struct commit *old_head,
+ const struct object_id *new_head,
+
[PATCH v4 8/9] sequencer: try to commit without forking 'git commit'
From: Phillip Wood
If the commit message does not need to be edited then create the
commit without forking 'git commit'. Taking the best time of ten runs
with a warm cache this reduces the time taken to cherry-pick 10
commits by 27% (from 282ms to 204ms), and the time taken by 'git
rebase --continue' to pick 10 commits by 45% (from 386ms to 212ms) on
my computer running linux. Some of greater saving for rebase is
because it no longer wastes time creating the commit summary just to
throw it away.
The code to create the commit is based on builtin/commit.c. It is
simplified as it doesn't have to deal with merges and modified so that
it does not die but returns an error to make sure the sequencer exits
cleanly, as it would when forking 'git commit'
Even when not forking 'git commit' the commit message is written to a
file and CHERRY_PICK_HEAD is created unnecessarily. This could be
eliminated in future. I hacked up a version that does not write these
files and just passed an strbuf (with the wrong message for fixup and
squash commands) to do_commit() but I couldn't measure any significant
time difference when running cherry-pick or rebase. I think
eliminating the writes properly for rebase would require a bit of
effort as the code would need to be restructured.
Signed-off-by: Phillip Wood
---
Notes:
changes since v3:
- take account of change print_commit_summary() return type after
dropping the patch that made it return an error instead of dying.
changes since v2:
- style fixes
changes since v1:
- added comments to explain return value of try_to_commit()
- removed unnecessary NULL tests before calling free()
- style cleanups
- corrected commit message
- prefixed cleanup_mode constants to reflect the changes to patch 2
in this series
sequencer.c | 178 +++-
1 file changed, 176 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index
40461a41e3798e267813656de6b669c297b521c6..99095a28c6732ef697b5b763b347751bd8a440cf
100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -592,6 +592,18 @@ static int read_env_script(struct argv_array *env)
return 0;
}
+static char *get_author(const char *message)
+{
+ size_t len;
+ const char *a;
+
+ a = find_commit_header(message, "author", &len);
+ if (a)
+ return xmemdupz(a, len);
+
+ return NULL;
+}
+
static const char staged_changes_advice[] =
N_("you have staged changes in your working tree\n"
"If these changes are meant to be squashed into the previous commit, run:\n"
@@ -984,6 +996,160 @@ void print_commit_summary(const char *prefix, const
struct object_id *oid,
strbuf_release(&format);
}
+static int parse_head(struct commit **head)
+{
+ struct commit *current_head;
+ struct object_id oid;
+
+ if (get_oid("HEAD", &oid)) {
+ current_head = NULL;
+ } else {
+ current_head = lookup_commit_reference(&oid);
+ if (!current_head)
+ return error(_("could not parse HEAD"));
+ if (oidcmp(&oid, ¤t_head->object.oid)) {
+ warning(_("HEAD %s is not a commit!"),
+ oid_to_hex(&oid));
+ }
+ if (parse_commit(current_head))
+ return error(_("could not parse HEAD commit"));
+ }
+ *head = current_head;
+
+ return 0;
+}
+
+/*
+ * Try to commit without forking 'git commit'. In some cases we need
+ * to run 'git commit' to display an error message
+ *
+ * Returns:
+ * -1 - error unable to commit
+ * 0 - success
+ * 1 - run 'git commit'
+ */
+static int try_to_commit(struct strbuf *msg, const char *author,
+struct replay_opts *opts, unsigned int flags,
+struct object_id *oid)
+{
+ struct object_id tree;
+ struct commit *current_head;
+ struct commit_list *parents = NULL;
+ struct commit_extra_header *extra = NULL;
+ struct strbuf err = STRBUF_INIT;
+ struct strbuf amend_msg = STRBUF_INIT;
+ char *amend_author = NULL;
+ const char *gpg_sign;
+ enum commit_msg_cleanup_mode cleanup;
+ int res = 0;
+
+ if (parse_head(¤t_head))
+ return -1;
+
+ if (flags & AMEND_MSG) {
+ const char *exclude_gpgsig[] = { "gpgsig", NULL };
+ const char *out_enc = get_commit_output_encoding();
+ const char *message = logmsg_reencode(current_head, NULL,
+ out_enc);
+
+ if (!msg) {
+ const char *orig_message = NULL;
+
+ find_commit_subject(message, &orig_message);
+ msg = &amend_msg;
+ strbuf_addstr(msg, orig_message);
+ }
+ author = ame
[PATCH v4 7/9] sequencer: load commit related config
From: Phillip Wood
Load default values for message cleanup and gpg signing of commits in
preparation for committing without forking 'git commit'. Note that we
interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE to
be consistent with 'git commit'
Signed-off-by: Phillip Wood
---
Notes:
changes since v3:
- interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE
to match 'git commit'
changes since v1:
- renamed git_revert_config() to common_config()
- prefixed cleanup_mode constants to reflect the changes to patch 2
in this series
builtin/rebase--helper.c | 13 -
builtin/revert.c | 15 +--
sequencer.c | 34 ++
sequencer.h | 1 +
4 files changed, 60 insertions(+), 3 deletions(-)
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index
f8519363a393862b6857acab037e74367c7f2134..68194d3aed950f327a8bc624fa1991478dfea01e
100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -9,6 +9,17 @@ static const char * const builtin_rebase_helper_usage[] = {
NULL
};
+static int git_rebase_helper_config(const char *k, const char *v, void *cb)
+{
+ int status;
+
+ status = git_sequencer_config(k, v, NULL);
+ if (status)
+ return status;
+
+ return git_default_config(k, v, NULL);
+}
+
int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
{
struct replay_opts opts = REPLAY_OPTS_INIT;
@@ -39,7 +50,7 @@ int cmd_rebase__helper(int argc, const char **argv, const
char *prefix)
OPT_END()
};
- git_config(git_default_config, NULL);
+ git_config(git_rebase_helper_config, NULL);
opts.action = REPLAY_INTERACTIVE_REBASE;
opts.allow_ff = 1;
diff --git a/builtin/revert.c b/builtin/revert.c
index
b9d927eb09c9ed87c84681df1396f4e6d9b13c97..1938825efa6ad20ede5aba57f097863aeb33d1d5
100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -31,6 +31,17 @@ static const char * const cherry_pick_usage[] = {
NULL
};
+static int common_config(const char *k, const char *v, void *cb)
+{
+ int status;
+
+ status = git_sequencer_config(k, v, NULL);
+ if (status)
+ return status;
+
+ return git_default_config(k, v, NULL);
+}
+
static const char *action_name(const struct replay_opts *opts)
{
return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
@@ -208,7 +219,7 @@ int cmd_revert(int argc, const char **argv, const char
*prefix)
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
- git_config(git_default_config, NULL);
+ git_config(common_config, NULL);
res = run_sequencer(argc, argv, &opts);
if (res < 0)
die(_("revert failed"));
@@ -221,7 +232,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char
*prefix)
int res;
opts.action = REPLAY_PICK;
- git_config(git_default_config, NULL);
+ git_config(common_config, NULL);
res = run_sequencer(argc, argv, &opts);
if (res < 0)
die(_("cherry-pick failed"));
diff --git a/sequencer.c b/sequencer.c
index
7400df5522037583108534755af6f542117667c2..40461a41e3798e267813656de6b669c297b521c6
100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -688,6 +688,40 @@ static int run_git_commit(const char *defmsg, struct
replay_opts *opts,
return run_command(&cmd);
}
+static enum commit_msg_cleanup_mode default_msg_cleanup =
+ COMMIT_MSG_CLEANUP_NONE;
+static char *default_gpg_sign;
+
+int git_sequencer_config(const char *k, const char *v, void *cb)
+{
+ if (!strcmp(k, "commit.cleanup")) {
+ int status;
+ const char *s;
+
+ status = git_config_string(&s, k, v);
+ if (status)
+ return status;
+
+ if (!strcmp(s, "verbatim"))
+ default_msg_cleanup = COMMIT_MSG_CLEANUP_NONE;
+ else if (!strcmp(s, "whitespace"))
+ default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
+ else if (!strcmp(s, "strip"))
+ default_msg_cleanup = COMMIT_MSG_CLEANUP_ALL;
+ else if (!strcmp(s, "scissors"))
+ default_msg_cleanup = COMMIT_MSG_CLEANUP_SPACE;
+
+ return status;
+ }
+
+ if (!strcmp(k, "commit.gpgsign")) {
+ default_gpg_sign = git_config_bool(k, v) ? "" : NULL;
+ return 0;
+ }
+
+ return git_gpg_config(k, v, NULL);
+}
+
static int rest_is_empty(const struct strbuf *sb, int start)
{
int i, eol;
diff --git a/sequencer.h b/sequencer.h
index
4f616c61a3f3869daf9f427b978c308d6094a978..77cb174b2aaf3972ebb9e6ec379252be96dedd3d
100644
--- a/sequencer.h
+++ b/sequencer.h
@@
[PATCH v4 9/9] t3512/t3513: remove KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1
From: Phillip Wood Now that the sequencer creates commits without forking 'git commit' it does not see an empty commit in these tests which fixes the known breakage. Note that logic for handling KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1 is not removed from lib-submodule-update.sh as it is still used by other tests. Signed-off-by: Phillip Wood --- Notes: The output of the tests with after the previous patch looks like the output of the merge tests (see below), so I'm hopeful that this is a genuine fix, but someone who knows about submodules should check that things are in fact working properly now. t3512-cherry-pick-submodule.sh expecting success: prolog && reset_work_tree_to add_sub1 && ( cd submodule_update && git branch -t modify_sub1 origin/modify_sub1 && $command modify_sub1 && test_superproject_content origin/modify_sub1 && test_submodule_content sub1 origin/add_sub1 && git submodule update && test_submodule_content sub1 origin/modify_sub1 ) Cloning into 'submodule_update'... done. Switched to a new branch 'add_sub1' Branch 'add_sub1' set up to track remote branch 'add_sub1' from 'origin'. Submodule 'sub1' (/home/phil/Documents/src/git/t/trash directory.t3512-cherry-pick-submodule/submodule_update_sub1) registered for path 'sub1' Cloning into '/home/phil/Documents/src/git/t/trash directory.t3512-cherry-pick-submodule/submodule_update/sub1'... done. Submodule path 'sub1': checked out 'ce9efb76b6ff5beb56e70d3662695f3ecbd38330' Branch 'modify_sub1' set up to track remote branch 'modify_sub1' from 'origin'. [add_sub1 e57a25c] Modify sub1 Author: A U Thor Date: Fri Nov 24 10:48:45 2017 + Submodule path 'sub1': checked out '7c9cd6d138a7bb3145fc0c7fca1f403cbe89010e' ok 11 - git cherry-pick: modified submodule does not update submodule work tree expecting success: prolog && reset_work_tree_to add_sub1 && ( cd submodule_update && git branch -t invalid_sub1 origin/invalid_sub1 && $command invalid_sub1 && test_superproject_content origin/invalid_sub1 && test_submodule_content sub1 origin/add_sub1 && test_must_fail git submodule update && test_submodule_content sub1 origin/add_sub1 ) Cloning into 'submodule_update'... done. Switched to a new branch 'add_sub1' Branch 'add_sub1' set up to track remote branch 'add_sub1' from 'origin'. Submodule 'sub1' (/home/phil/Documents/src/git/t/trash directory.t3512-cherry-pick-submodule/submodule_update_sub1) registered for path 'sub1' Cloning into '/home/phil/Documents/src/git/t/trash directory.t3512-cherry-pick-submodule/submodule_update/sub1'... done. Submodule path 'sub1': checked out 'ce9efb76b6ff5beb56e70d3662695f3ecbd38330' Branch 'invalid_sub1' set up to track remote branch 'invalid_sub1' from 'origin'. [add_sub1 155695c] Invalid sub1 commit Author: A U Thor Date: Fri Nov 24 10:48:45 2017 + error: Server does not allow request for unadvertised object 0123456789012345678901234567890123456789 Fetched in submodule path 'sub1', but it did not contain 0123456789012345678901234567890123456789. Direct fetching of that commit failed. ok 12 - git cherry-pick: modified submodule does not update submodule work tree to invalid commit expecting success: prolog && reset_work_tree_to invalid_sub1 && ( cd submodule_update && git branch -t valid_sub1 origin/valid_sub1 && $command valid_sub1 && test_superproject_content origin/valid_sub1 && test_dir_is_empty sub1 && git submodule update --init --recursive && test_submodule_content sub1 origin/valid_sub1 ) Cloning into 'submodule_update'... done. Switched to a new branch 'invalid_sub1' Branch 'invalid_sub1' set up to track remote branch 'invalid_sub1' from 'origin'. fatal: Needed a single revision Branch 'valid_sub1' set up to track remote branch 'valid_sub1' from 'origin'. [invalid_sub1 497299e] Revert "Invalid sub1 commit" Author: A U Thor Date: Fri Nov 24 10:48:46 2017 + Submodule 'sub1' (/home/phil/Documents/src/git/t/trash directory.t3512-cherry-pick-submodule/submodule_update_sub1) registered for path 'sub1' Submodule 'uninitialized_sub' (/home/phil/Documents/src/git/t/trash di
[PATCH v4 4/9] commit: move post-rewrite code to libgit
From: Phillip Wood
Move run_rewrite_hook() from bulitin/commit.c to sequencer.c so it can
be shared with other commands and add a new function
commit_post_rewrite() based on the code in builtin/commit.c that
encapsulates rewriting notes and running the post-rewrite hook. Once
the sequencer learns how to create commits without forking 'git
commit' these functions will be used when squashing commits.
Signed-off-by: Phillip Wood
---
Notes:
changes since v1:
- reword commit message to explain why the code is being moved
builtin/commit.c | 42 +-
sequencer.c | 47 +++
sequencer.h | 2 ++
3 files changed, 50 insertions(+), 41 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index
eb144556bf37b7bf357bd976b94305171b4fd159..d251cfcebad3476c365492d83803e7821fdfdf2b
100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -31,9 +31,7 @@
#include "gpg-interface.h"
#include "column.h"
#include "sequencer.h"
-#include "notes-utils.h"
#include "mailmap.h"
-#include "sigchain.h"
static const char * const builtin_commit_usage[] = {
N_("git commit [] [--] ..."),
@@ -1497,37 +1495,6 @@ static int git_commit_config(const char *k, const char
*v, void *cb)
return git_status_config(k, v, s);
}
-static int run_rewrite_hook(const struct object_id *oldoid,
- const struct object_id *newoid)
-{
- struct child_process proc = CHILD_PROCESS_INIT;
- const char *argv[3];
- int code;
- struct strbuf sb = STRBUF_INIT;
-
- argv[0] = find_hook("post-rewrite");
- if (!argv[0])
- return 0;
-
- argv[1] = "amend";
- argv[2] = NULL;
-
- proc.argv = argv;
- proc.in = -1;
- proc.stdout_to_stderr = 1;
-
- code = start_command(&proc);
- if (code)
- return code;
- strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
- sigchain_push(SIGPIPE, SIG_IGN);
- write_in_full(proc.in, sb.buf, sb.len);
- close(proc.in);
- strbuf_release(&sb);
- sigchain_pop(SIGPIPE);
- return finish_command(&proc);
-}
-
int run_commit_hook(int editor_is_used, const char *index_file, const char
*name, ...)
{
struct argv_array hook_env = ARGV_ARRAY_INIT;
@@ -1758,14 +1725,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
rerere(0);
run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
if (amend && !no_post_rewrite) {
- struct notes_rewrite_cfg *cfg;
- cfg = init_copy_notes_for_rewrite("amend");
- if (cfg) {
- /* we are amending, so current_head is not NULL */
- copy_note_for_rewrite(cfg, ¤t_head->object.oid,
&oid);
- finish_copy_notes_for_rewrite(cfg, "Notes added by 'git
commit --amend'");
- }
- run_rewrite_hook(¤t_head->object.oid, &oid);
+ commit_post_rewrite(current_head, &oid);
}
if (!quiet)
print_summary(prefix, &oid, !current_head);
diff --git a/sequencer.c b/sequencer.c
index
ef262980c5255d90ee023c0b29c6c1c628b3c7d2..6bc8346d42bb3cb1d2dc6a2238dd1b38e4308914
100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -21,6 +21,8 @@
#include "log-tree.h"
#include "wt-status.h"
#include "hashmap.h"
+#include "notes-utils.h"
+#include "sigchain.h"
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -789,6 +791,51 @@ int update_head_with_reflog(const struct commit *old_head,
return ret;
}
+static int run_rewrite_hook(const struct object_id *oldoid,
+ const struct object_id *newoid)
+{
+ struct child_process proc = CHILD_PROCESS_INIT;
+ const char *argv[3];
+ int code;
+ struct strbuf sb = STRBUF_INIT;
+
+ argv[0] = find_hook("post-rewrite");
+ if (!argv[0])
+ return 0;
+
+ argv[1] = "amend";
+ argv[2] = NULL;
+
+ proc.argv = argv;
+ proc.in = -1;
+ proc.stdout_to_stderr = 1;
+
+ code = start_command(&proc);
+ if (code)
+ return code;
+ strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
+ sigchain_push(SIGPIPE, SIG_IGN);
+ write_in_full(proc.in, sb.buf, sb.len);
+ close(proc.in);
+ strbuf_release(&sb);
+ sigchain_pop(SIGPIPE);
+ return finish_command(&proc);
+}
+
+void commit_post_rewrite(const struct commit *old_head,
+const struct object_id *new_head)
+{
+ struct notes_rewrite_cfg *cfg;
+
+ cfg = init_copy_notes_for_rewrite("amend");
+ if (cfg) {
+ /* we are amending, so old_head is not NULL */
+ copy_note_for_rewrite(cfg, &old_head->object.oid, new_head);
+ finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commi
[PATCH v4 2/9] commit: move empty message checks to libgit
From: Phillip Wood
Move the functions that check for empty messages from bulitin/commit.c
to sequencer.c so they can be shared with other commands. The
functions are refactored to take an explicit cleanup mode and template
filename passed by the caller.
Signed-off-by: Phillip Wood
---
Notes:
changes since v1:
- prefix cleanup_mode enum and constants with commit_msg_
builtin/commit.c | 99 +++-
sequencer.c | 61 ++
sequencer.h | 11 +++
3 files changed, 91 insertions(+), 80 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index
8a877014145435516930c787dec37b8c4ac3da90..d958c2eb2adc9a29dab29340ce9b56daea41fecd
100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -128,12 +128,7 @@ static char *sign_commit;
* if editor is used, and only the whitespaces if the message
* is specified explicitly.
*/
-static enum {
- CLEANUP_SPACE,
- CLEANUP_NONE,
- CLEANUP_SCISSORS,
- CLEANUP_ALL
-} cleanup_mode;
+static enum commit_msg_cleanup_mode cleanup_mode;
static const char *cleanup_arg;
static enum commit_whence whence;
@@ -673,7 +668,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
struct strbuf sb = STRBUF_INIT;
const char *hook_arg1 = NULL;
const char *hook_arg2 = NULL;
- int clean_message_contents = (cleanup_mode != CLEANUP_NONE);
+ int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE);
int old_display_comment_prefix;
/* This checks and barfs if author is badly specified */
@@ -812,7 +807,7 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
struct ident_split ci, ai;
if (whence != FROM_COMMIT) {
- if (cleanup_mode == CLEANUP_SCISSORS)
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
wt_status_add_cut_line(s->fp);
status_printf_ln(s, GIT_COLOR_NORMAL,
whence == FROM_MERGE
@@ -832,14 +827,15 @@ static int prepare_to_commit(const char *index_file,
const char *prefix,
}
fprintf(s->fp, "\n");
- if (cleanup_mode == CLEANUP_ALL)
+ if (cleanup_mode == COMMIT_MSG_CLEANUP_ALL)
status_printf(s, GIT_COLOR_NORMAL,
_("Please enter the commit message for your
changes."
" Lines starting\nwith '%c' will be ignored,
and an empty"
" message aborts the commit.\n"),
comment_line_char);
- else if (cleanup_mode == CLEANUP_SCISSORS && whence ==
FROM_COMMIT)
+ else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
+whence == FROM_COMMIT)
wt_status_add_cut_line(s->fp);
- else /* CLEANUP_SPACE, that is. */
+ else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
status_printf(s, GIT_COLOR_NORMAL,
_("Please enter the commit message for your
changes."
" Lines starting\n"
@@ -984,65 +980,6 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
return 1;
}
-static int rest_is_empty(struct strbuf *sb, int start)
-{
- int i, eol;
- const char *nl;
-
- /* Check if the rest is just whitespace and Signed-off-by's. */
- for (i = start; i < sb->len; i++) {
- nl = memchr(sb->buf + i, '\n', sb->len - i);
- if (nl)
- eol = nl - sb->buf;
- else
- eol = sb->len;
-
- if (strlen(sign_off_header) <= eol - i &&
- starts_with(sb->buf + i, sign_off_header)) {
- i = eol;
- continue;
- }
- while (i < eol)
- if (!isspace(sb->buf[i++]))
- return 0;
- }
-
- return 1;
-}
-
-/*
- * Find out if the message in the strbuf contains only whitespace and
- * Signed-off-by lines.
- */
-static int message_is_empty(struct strbuf *sb)
-{
- if (cleanup_mode == CLEANUP_NONE && sb->len)
- return 0;
- return rest_is_empty(sb, 0);
-}
-
-/*
- * See if the user edited the message in the editor or left what
- * was in the template intact
- */
-static int template_untouched(struct strbuf *sb)
-{
- struct strbuf tmpl = STRBUF_INIT;
- const char *start;
-
- if (cleanup_mode == CLEANUP_NONE && sb->len)
- return 0;
-
- if (!template_file || strbuf_read_file(&tmpl, template_file, 0) <= 0)
- return 0;
-
- strbuf_stripspace(&tmpl, cleanup_mode == CLEANUP_ALL);
- if (!skip_prefix(sb->buf
[PATCH v4 0/9] sequencer: don't fork git commit
From: Phillip Wood I've updated the patches to fix the embarassing build failure in v3. I've also added a patch to remove the known breakage from some of the tests in t3512/t3513 that now pass - someone who knows about submodules should check this. The only other change is to interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE when loading the default value for message cleanups to be consistent with 'git commit'. (I can't imagine many people have that value set in their config) Here's the original summary: These patches teach the sequencer to create commits without forking git commit when the commit message does not need to be edited. This speeds up cherry picking 10 commits by 26% and picking 10 commits with rebase --continue by 44%. The first few patches move bits of builtin/commit.c to sequencer.c. The last two patches actually implement creating commits in sequencer.c. Phillip Wood (9): t3404: check intermediate squash messages commit: move empty message checks to libgit Add a function to update HEAD after creating a commit commit: move post-rewrite code to libgit commit: move print_commit_summary() to libgit sequencer: simplify adding Signed-off-by: trailer sequencer: load commit related config sequencer: try to commit without forking 'git commit' t3512/t3513: remove KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1 builtin/commit.c | 289 +++ builtin/rebase--helper.c | 13 +- builtin/revert.c | 15 +- sequencer.c | 486 ++- sequencer.h | 23 ++ t/t3404-rebase-interactive.sh| 4 + t/t3512-cherry-pick-submodule.sh | 1 - t/t3513-revert-submodule.sh | 1 - 8 files changed, 561 insertions(+), 271 deletions(-) -- 2.15.0
[PATCH v4 6/9] sequencer: simplify adding Signed-off-by: trailer
From: Phillip Wood
Add the Signed-off-by: trailer in one place rather than adding it to
the message when doing a recursive merge and specifying '--signoff'
when running 'git commit'. This means that if there are conflicts when
merging with a strategy other than 'recursive' the Signed-off-by:
trailer will be added if the user commits the resolution themselves
without passing '--signoff' to 'git commit'. It also simplifies the
in-process commit that is about to be added to the sequencer.
Signed-off-by: Phillip Wood
---
sequencer.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index
a2cf6f5e06ffec5108f0faf43d1a4cb605264c3f..7400df5522037583108534755af6f542117667c2
100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -477,9 +477,6 @@ static int do_recursive_merge(struct commit *base, struct
commit *next,
_(action_name(opts)));
rollback_lock_file(&index_lock);
- if (opts->signoff)
- append_signoff(msgbuf, 0, 0);
-
if (!clean)
append_conflicts_hint(msgbuf);
@@ -657,8 +654,6 @@ static int run_git_commit(const char *defmsg, struct
replay_opts *opts,
argv_array_push(&cmd.args, "--amend");
if (opts->gpg_sign)
argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
- if (opts->signoff)
- argv_array_push(&cmd.args, "-s");
if (defmsg)
argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
if ((flags & CLEANUP_MSG))
@@ -1347,6 +1342,9 @@ static int do_pick_commit(enum todo_command command,
struct commit *commit,
}
}
+ if (opts->signoff)
+ append_signoff(&msgbuf, 0, 0);
+
if (is_rebase_i(opts) && write_author_script(msg.message) < 0)
res = -1;
else if (!opts->strategy || !strcmp(opts->strategy, "recursive") ||
command == TODO_REVERT) {
--
2.15.0
[PATCH v4 1/9] t3404: check intermediate squash messages
From: Phillip Wood
When there is more than one squash/fixup command in a row check the
intermediate messages are correct.
Signed-off-by: Phillip Wood
---
t/t3404-rebase-interactive.sh | 4
1 file changed, 4 insertions(+)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index
6a82d1ed876dd5d1073dc63be8ba5720adbf12e3..9ed0a244e6cdf34c7caca8232f0c0a8cf4864c42
100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -453,6 +453,10 @@ test_expect_success C_LOCALE_OUTPUT 'squash and fixup
generate correct log messa
git rebase -i $base &&
git cat-file commit HEAD | sed -e 1,/^\$/d > actual-squash-fixup &&
test_cmp expect-squash-fixup actual-squash-fixup &&
+ git cat-file commit HEAD@{2} |
+ grep "^# This is a combination of 3 commits\." &&
+ git cat-file commit HEAD@{3} |
+ grep "^# This is a combination of 2 commits\." &&
git checkout to-be-rebased &&
git branch -D squash-fixup
'
--
2.15.0
Re: [PATCH v3] doc: tweak "man gitcli" for clarity
On Fri, 24 Nov 2017, Junio C Hamano wrote: > "Robert P. J. Day" writes: > > > -This manual describes the convention used throughout Git CLI. > > +This manual describes the conventions used throughout Git CLI. > > OK. > > > Many commands take revisions (most often "commits", but sometimes > > "tree-ish", depending on the context and command) and paths as their > > @@ -32,32 +32,35 @@ arguments. Here are the rules: > > between the HEAD commit and the work tree as a whole". You can say > > `git diff HEAD --` to ask for the latter. > > > > - * Without disambiguating `--`, Git makes a reasonable guess, but errors > > - out and asking you to disambiguate when ambiguous. E.g. if you have a > > - file called HEAD in your work tree, `git diff HEAD` is ambiguous, and > > + * In cases where a Git command is truly ambiguous, Git will error out > > + and ask you to disambiguate the command. E.g. if you have a file > > + called HEAD in your work tree, `git diff HEAD` is ambiguous, and > > you have to say either `git diff HEAD --` or `git diff -- HEAD` to > > disambiguate. > > + > > When writing a script that is expected to handle random user-input, it is > > a good practice to make it explicit which arguments are which by placing > > -disambiguating `--` at appropriate places. > > +a disambiguating `--` at appropriate places. > > The above "truly" is misleading by giving the information the other > way around. We ask to disambiguate when we cannot readily say the > command line is *not* unambiguous. That is different from asking > when we know it is truly ambiguous. > > Also see if you want > to know more. ... snip ... at this point, i would throw out *all* of this and, rather than try to cram disambiguation into the bullet lists currently in that man page, just break it out into its own section, where it can be explained properly and comprehensively. the reason this has gotten so chaotic is that we're trying to preserve the structure that is in that man page, when it should just be tossed and rewritten to give "--" and disambiguation the section it deserves. rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: [PATCH v2 1/9] perf/run: add '--config' option to the 'run' script
On Mon, Oct 16 2017, Junio C. Hamano jotted: > Christian Couder writes: > >> It is error prone and tiring to use many long environment >> variables to give parameters to the 'run' script. > > This topic has been sitting in the list archive without getting much > reaction from list participants. Is anybody happy with these > patches? Very late reply, sorry. But I've looked these over and they look good to me.
Re: Re: Unify annotated and non-annotated tags
On Fri, Nov 24, 2017 at 10:52 AM, anatoly techtonik wrote: > On Thu, Nov 23, 2017 at 6:08 PM, Randall S. Becker > wrote: >> On 2017-11-23 02:31 (GMT-05:00) anatoly techtonik wrote >>>Subject: Re: Unify annotated and non-annotated tags >>>On Sat, Nov 11, 2017 at 5:06 AM, Junio C Hamano wrote: Igor Djordjevic writes: > If you would like to mimic output of "git show-ref", repeating > commits for each tag pointing to it and showing full tag name as > well, you could do something like this, for example: > > for tag in $(git for-each-ref --format="%(refname)" refs/tags) > do > printf '%s %s\n' "$(git rev-parse $tag^0)" "$tag" > done > > > Hope that helps a bit. If you use for-each-ref's --format option, you could do something like (pardon a long line): git for-each-ref --format='%(if)%(*objectname)%(then)%(*objectname)%(else)%(objectname)%(end) %(refname)' refs/tags without any loop, I would think. >>>Thanks. That helps. >>>So my proposal is to get rid of non-annotated tags, so to get all >>>tags with commits that they point to, one would use: >>>git for-each-ref --format='%(*objectname) %(refname)' refs/tags> >>>For so-called non-annotated tags just leave the message empty. >>>I don't see why anyone would need non-annotated tags though. >> >> I have seen non-annotated tags used in automations (not necessarily well >> written ones) that create tags as a record of automation activity. I am not >> sure we should be writing off the concept of unannotated tags entirely. This >> may cause breakage based on existing expectations of how tags work at >> present. My take is that tags should include whodunnit, even if it's just >> the version of the automation being used, but I don't always get to have my >> wishes fulfilled. In essence, whatever behaviour a non-annotated tag has now >> may need to be emulated in future even if reconciliation happens. An option >> to preserve empty tag compatibility with pre-2.16 behaviour, perhaps? Sadly, >> I cannot supply examples of this usage based on a human memory page-fault >> and NDAs. > > Are there any windows for backward compatibility breaks, or git is > doomed to preserve it forever? > Automation without support won't survive for long, and people who rely > on that, like Chromium team, usually hard set the version used. Git is not doomed to preserve anything forever. We've gradually broken backwards compatibility for a few core things like these. However, just as a bystander reading this thread I haven't seen any compelling reason for why these should be removed. You initially had questions about how to extract info about them, which you got answers to. So what reasons remain for why they need to be removed?
Re: Re: Unify annotated and non-annotated tags
On Thu, Nov 23, 2017 at 6:08 PM, Randall S. Becker wrote: > On 2017-11-23 02:31 (GMT-05:00) anatoly techtonik wrote >>Subject: Re: Unify annotated and non-annotated tags >>On Sat, Nov 11, 2017 at 5:06 AM, Junio C Hamano wrote: >>> Igor Djordjevic writes: >>> If you would like to mimic output of "git show-ref", repeating commits for each tag pointing to it and showing full tag name as well, you could do something like this, for example: for tag in $(git for-each-ref --format="%(refname)" refs/tags) do printf '%s %s\n' "$(git rev-parse $tag^0)" "$tag" done Hope that helps a bit. >>> >>> If you use for-each-ref's --format option, you could do something >>> like (pardon a long line): >>> >>> git for-each-ref >>> --format='%(if)%(*objectname)%(then)%(*objectname)%(else)%(objectname)%(end) >>> %(refname)' refs/tags >>> >>> without any loop, I would think. >>Thanks. That helps. >>So my proposal is to get rid of non-annotated tags, so to get all >>tags with commits that they point to, one would use: >>git for-each-ref --format='%(*objectname) %(refname)' refs/tags> >>For so-called non-annotated tags just leave the message empty. >>I don't see why anyone would need non-annotated tags though. > > I have seen non-annotated tags used in automations (not necessarily well > written ones) that create tags as a record of automation activity. I am not > sure we should be writing off the concept of unannotated tags entirely. This > may cause breakage based on existing expectations of how tags work at > present. My take is that tags should include whodunnit, even if it's just the > version of the automation being used, but I don't always get to have my > wishes fulfilled. In essence, whatever behaviour a non-annotated tag has now > may need to be emulated in future even if reconciliation happens. An option > to preserve empty tag compatibility with pre-2.16 behaviour, perhaps? Sadly, > I cannot supply examples of this usage based on a human memory page-fault and > NDAs. Are there any windows for backward compatibility breaks, or git is doomed to preserve it forever? Automation without support won't survive for long, and people who rely on that, like Chromium team, usually hard set the version used. -- anatoly t.
Re: [PATCH v2] doc: clarify that "git bisect" accepts one or more good commits
On Fri, 24 Nov 2017, Junio C Hamano wrote: > "Robert P. J. Day" writes: ... snip ... > > -to indicate that it was after. > > +to indicate a single commit after that change. > > As to "identify", I would say it is better to consistently use > "indicate" like the original of these two hunks at the end says, > i.e. "indicate that it is bad/new (or they are good/old)". i'm going to ponder this, but let me explain why i am such an annoying stickler for the choice of words at times, and you can read all about it here: http://twain.lib.virginia.edu/projects/rissetto/offense.html in particular, i draw your attention to twain's trashing of cooper for, in many cases, using *almost* the right word, but not *quite* the right word: "Cooper's word-sense was singularly dull. When a person has a poor ear for music he will flat and sharp right along without knowing it. He keeps near the tune, but is not the tune. When a person has a poor ear for words, the result is a literary flatting and sharping; you perceive what he is intending to say, but you also perceive that he does not say it. This is Cooper. He was not a word-musician. His ear was satisfied with the approximate words. I will furnish some circumstantial evidence in support of this charge. My instances are gathered from half a dozen pages of the tale called "Deerslayer." He uses "Verbal" for "oral"; "precision" for "facility"; "phenomena" for "marvels"; "necessary" for "predetermined"; "unsophisticated" for "primitive"; "preparation" for "expectancy"; "rebuked" for "subdued"; "dependent on" for "resulting from"; "fact" for "condition"; "fact" for "conjecture"; "precaution" for "caution"; "explain" for "determine"; "mortified" for "disappointed"; "meretricious" for "factitious"; "materially" for "considerably"; "decreasing" for "deepening"; "increasing" for "disappearing"; "embedded" for "inclosed"; "treacherous" for "hostile"; "stood" for "stooped"; "softened" for "replaced"; "rejoined" for "remarked"; "situation" for "condition"; "different" for "differing"; "insensible" for "unsentient"; "brevity" for "celerity"; "distrusted" for "suspicious"; "mental imbecility" for "imbecility"; "eyes" for "sight"; "counteracting" for "opposing"; "funeral obsequies" for "obsequies." in this sense, i don't think "indicate" and "identify" are completely interchangeable. in my mind, the word "identify" does nothing more than, you know, point at something and say, "that one, that's the one i'm talking about;" it goes no further than that. on the other hand, the word "indicate" (in my mind) implies that you're about to provide some *property* or *quality* of something, and you do exactly that in the earlier quote: > As to "identify", I would say it is better to consistently use > "indicate" like the original of these two hunks at the end says, > i.e. "indicate that it is bad/new (or they are good/old)". as in, you "identify" a commit, but you "indicate" that it represents a good or bad commit. i know this sounds picky, but it is exactly this tendency of using *almost* the right word that makes a lot of documentation potentially confusing. given this distinction, depending on the word to be used, i would write one of: 1) "first, identify the bad commit and one or more good commits..." 2) "first, indicate which commit is the bad commit, and which commits are the good commits ..." the eventual meaning *should* be the same, but the choice of words can make the meaning clear, or can confuse the reader. thoughts? rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
AW: [PATCH] http-backend: respect CONTENT_LENGTH as specified by rfc3875
Hi All, thx Max for jumping in, I wasn't able to complete this due to serious lack of time. Later I forgot it. Great to see that this finally made it. Mit freundlichen Grüßen / With kind regards Florian Manschwetus E-Mail: manschwe...@cs-software-gmbh.de Tel.: +49-(0)611-8908534 CS Software Concepts and Solutions GmbH Geschäftsführer / Managing director: Dr. Werner Alexi Amtsgericht Wiesbaden HRB 10004 (Commercial registry) Schiersteiner Straße 31 D-65187 Wiesbaden Germany Tel.: 0611/8908555 > -Ursprüngliche Nachricht- > Von: Junio C Hamano [mailto:gits...@pobox.com] > Gesendet: Freitag, 24. November 2017 06:55 > An: Max Kirillov > Cc: Jeff King; Florian Manschwetus; Chris Packham; Konstantin Khomoutov; > git@vger.kernel.org > Betreff: Re: [PATCH] http-backend: respect CONTENT_LENGTH as specified > by rfc3875 > > Max Kirillov writes: > > > http-backend reads whole input until EOF. However, the RFC 3875 > > specifies that a script must read only as many bytes as specified by > > CONTENT_LENGTH environment variable. This causes hang under > IIS/Windows, for example. > > > > Make http-backend read only CONTENT_LENGTH bytes, if it's defined, > > rather than the whole input until EOF. If the varibale is not defined, > > keep older behavior of reading until EOF because it is used to support > chunked transfer-encoding. > > > > Signed-off-by: Florian Manschwetus gmbh.de> > > Authored-by: Florian Manschwetus gmbh.de> > > Fixed-by: Max Kirillov > > Signed-off-by: Max Kirillov > > --- > > ... > > I hope I marked it correctly in the trailers. > > It is probably more conventional to do it like so: > > From: Florian Manschwetus > Date: > > http-backend reads whole input until EOF. However, the RFC 3875... > ... chunked transfer-encoding. > > Signed-off-by: Florian Manschwetus gmbh.de> > [mk: fixed trivial build failures and stuff] > Signed-off-by: Max Kirillov > --- > > > > > +/* > > + * replacement for original read_request, now renamed to > > +read_request_eof, > > + * honoring given content_length (req_len), > > + * provided by new wrapper function read_request */ > > I agree with Eric's suggestion. In-code comment is read by those who have > the current code, without knowing/caring what it used to be. "It used to do > this, but replace it with this new thing because..." is a valuable thing to > record > in the log message, but not here.
