[PATCH] Resending - Updated Bulgarian translation of git-gui
This is the updated Bulgarian translation og git-gui. It has been synced with git & gitk. I have just checked the archives of the mailing list - the patch was never sent, just the previous version of the cover letter. Resending so that it can be merged for 2.1. Kind regards: al_shopov Alexander Shopov (1): git-gui i18n: Updated Bulgarian translation (520t,0f,0u) po/bg.po | 3572 +++--- 1 file changed, 1781 insertions(+), 1791 deletions(-) -- 1.9.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Merging semi-parallel universes into one coherent repository
Hello I created a post on SO about this; pasting it here for convenience: = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = http://stackoverflow.com/questions/25193518/fixing-branches-disconnected-from-master After "completing" a repository migration, we noticed that all of our branches are all behind and ahead of master by hundreds to thousands of commits. Some context -- originally I used git-svn to migrate the repositories from SVN to git. Later down the line I used this solution to merge multiple (7) repositories into one. However, I also improvised on that solution for the repositories' branches. The result was disconnected branches -- branches that have all of the commits and proper change history, but are not actually associated with master in any way since the commits in the branches have different SHA's from those in the new, shared master. Additionally, the original "branching" in the separate repositories happened at different revisions, for example project X might have branched for release 1.0 at revision 30 whereas project Y might have branched fro release 1.0 at revision 42... and all of them have different times (and perhaps multiple times) where they merged back to master. Is there any clean way to fix this link to make it look like all of these repositories branched together? And to fix the commits so they actually properly map to commits in master? = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = I am sure there is a better way to word the above... but basically I have seven used-to-be-separate repositories with the same branches (say r1.0 as above). After some magic with filter-branch, they are all one repository -- all history in-tact. Additionally, same-named branches had their histories mashed together. That's all peachy. What's *not* peachy is that each of these branches shares commits with master and doesn't know it. Thus each branch looks "disconnected" from master, e.g. there is no pretty branch graph when you look through the history with "git log --pretty=format:"%h %s" --graph". Does anybody know of a way to rectify this issue? Let me know if any clarifications are needed. Help is much appreciated... -- Ryan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "Branch objects" (was: Re: cherry picking and merge)
On Thu, Aug 7, 2014 at 6:38 AM, Tony Finch wrote: > So I have a small tool which maintains a publication branch which tracks > the head of a rebasing branch. It's reasonably satisfactory so far... > > https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git > > ... though the structure of the publication branch is weird and not very > easy to navigate. You can see it in action in my git.git repo: You know, maybe you could even use this to automatically figure out the merge base for downstreams that follow your rebased branch: auto-generate the git rebase --onto . That would be awesome, particularly if integrated into git. It would then be fine to rebase published branches in most cases, for example. Nico -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "Branch objects" (was: Re: cherry picking and merge)
On Thu, Aug 7, 2014 at 12:47 PM, Tony Finch wrote: > Nico Williams wrote: >> Either way I retain the old HEAD with some name. > > Hmm, yes, I can see that would work. However my previous workflow was > rather branch-heavy and I found the accumulation of names annoying. I have > not yet had enough usage out of git-repub to see if it goes too far in the > direction of lack-of-names. A big omission is no opportunity to edit its > commit messages. Oh, I just read your script more carefully and looked at your example history again. You're using parent metadata in the commits to keep the history alive without the extra names, correct? *That* is _clever_. Hats off. I may have to steal this script :) Nico -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch wrote: > The to-do list commands `squash` and `fixup` apply the changes > introduced by the named commit to the tree but instead of creating > a new commit on top of the current head it replaces the previous > commit with a new commit that records the updated tree. If the > result is an empty commit git-rebase stops with the error message > >You asked to amend the most recent commit, but doing so would make >it empty. You can repeat your command with --allow-empty, or you can >remove the commit entirely with "git reset HEAD^". > > This message is not very helpful because neither does git-rebase > support an option `--allow-empty` nor does the messages say how to > resume the rebase. Firstly, change the error message to > >The squash result is empty and --keep-empty was not specified. > >You can remove the squash commit now with > > git reset HEAD^ > >Once you are down, run I guess you meant: s/down/done Same issue with the actually message in the code (below). > git rebase --continue > > If the user wishes to squash a sequence of commits into one > commit, f. i. > >pick A >squash Revert "A" >squash A' > > , it does not matter for the end result that the first squash > result, or any sub-sequence in general, is going to be empty. The > squash message is not affected at all by which commits are created > and only the commit created by the last line in the sequence will > end up in the final history. Secondly, print the error message > only if the whole squash sequence produced an empty commit. > > Lastly, since an empty squash commit is not a failure to rewrite > the history as planned, issue the message above as a mere warning > and interrupt the rebase with the return value zero. The > interruption should be considered as a notification with the > chance to undo it on the spot. Specifying the `--keep-empty` > option tells git-rebase to keep empty squash commits in the > rebased history without notification. > > Add tests. > > Reported-by: Peter Krefting > Signed-off-by: Fabian Ruch > --- > Hi, > > Peter Krefting is cc'd as the author of the bug report "Confusing > error message in rebase when commit becomes empty" discussed on the > mailing list in June. Phil Hord and Jeff King both participated in > the problem discussion which ended with two proposals by Jeff. > > Jeff King writes: >> 1. Always keep such empty commits. A user who is surprised by them >> being empty can then revisit them. Or drop them by doing another >> rebase without --keep-empty. >> >> 2. Notice ourselves that the end-result of the whole squash is an >> empty commit, and stop to let the user deal with it. > > This patch chooses the second alternative. Either way seems OK. The > crucial consensus of the discussion was to silently throw away empty > interim commits. > >Fabian > > git-rebase--interactive.sh| 20 +++--- > t/t3404-rebase-interactive.sh | 62 > +++ > 2 files changed, 79 insertions(+), 3 deletions(-) > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 3222bf6..8820eac 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -549,7 +549,7 @@ do_next () { > squash|s|fixup|f) > # This is an intermediate commit; its message will > only be > # used in case of trouble. So use the long version: > - do_with_author output git commit > --allow-empty-message \ > + do_with_author output git commit > --allow-empty-message --allow-empty \ > --amend --no-verify -F "$squash_msg" \ > ${gpg_sign_opt:+"$gpg_sign_opt"} || > die_failed_squash $sha1 "$rest" > @@ -558,18 +558,32 @@ do_next () { > # This is the final command of this squash/fixup group > if test -f "$fixup_msg" > then > - do_with_author git commit > --allow-empty-message \ > + do_with_author git commit > --allow-empty-message --allow-empty \ > --amend --no-verify -F "$fixup_msg" \ > ${gpg_sign_opt:+"$gpg_sign_opt"} || > die_failed_squash $sha1 "$rest" > else > cp "$squash_msg" "$GIT_DIR"/SQUASH_MSG || exit > rm -f "$GIT_DIR"/MERGE_MSG > - do_with_author git commit --amend --no-verify > -F "$GIT_DIR"/SQUASH_MSG -e \ > + do_with_author git commit --allow-empty > --amend --no-verify -F "$GIT_DIR"/SQUASH_MSG -e \ > ${gpg_sign
Re: [PATCH] stash: default listing to working-tree diff
Jeff King writes: > See the patch below, which I think could replace the top three from > jk/stash-list-p (or really, could replace the whole series, and the > bottom three could go into their own topic). Sounds sensible. Let's split those three changes into a separate topic (jk/pretty-empty-format) and queue this independently. Thanks. > -- >8 -- > Subject: stash: default listing to working-tree diff > > When you list stashes, you can provide arbitrary git-log > options to change the display. However, adding just "-p" > does nothing, because each stash is actually a merge commit. > > This implementation detail is easy to forget, leading to > confused users who think "-p" is not working. We can make > this easier by defaulting to "--first-parent -m", which will > show the diff against the working tree. This omits the > index portion of the stash entirely, but it's simple and it > matches what "git stash show" provides. > > People who are more clueful about stash's true form can use > "--cc" to override the "-m", and the "--first-parent" will > then do nothing. For diffs, it only affects non-combined > diffs, so "--cc" overrides it. And for the traversal, we are > walking the linear reflog anyway, so we do not even care > about the parents. > > Signed-off-by: Jeff King > --- > git-stash.sh | 2 +- > t/t3903-stash.sh | 42 ++ > 2 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/git-stash.sh b/git-stash.sh > index bcc757b..9c1ba8e 100755 > --- a/git-stash.sh > +++ b/git-stash.sh > @@ -297,7 +297,7 @@ have_stash () { > > list_stash () { > have_stash || return 0 > - git log --format="%gd: %gs" -g "$@" $ref_stash -- > + git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash -- > } > > show_stash () { > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 5b79b21..1e29962 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -685,4 +685,46 @@ test_expect_success 'handle stash specification with > spaces' ' > grep pig file > ' > > +test_expect_success 'setup stash with index and worktree changes' ' > + git stash clear && > + git reset --hard && > + echo index >file && > + git add file && > + echo working >file && > + git stash > +' > + > +test_expect_success 'stash list implies --first-parent -m' ' > + cat >expect <<-\EOF && > + stash@{0}: WIP on master: b27a2bc subdir > + > + diff --git a/file b/file > + index 257cc56..d26b33d 100644 > + --- a/file > + +++ b/file > + @@ -1 +1 @@ > + -foo > + +working > + EOF > + git stash list -p >actual && > + test_cmp expect actual > +' > + > +test_expect_success 'stash list --cc shows combined diff' ' > + cat >expect <<-\EOF && > + stash@{0}: WIP on master: b27a2bc subdir > + > + diff --cc file > + index 257cc56,9015a7a..d26b33d > + --- a/file > + +++ b/file > + @@@ -1,1 -1,1 +1,1 @@@ > + - foo > + -index > + ++working > + EOF > + git stash list -p --cc >actual && > + test_cmp expect actual > +' > + > test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/8] Rewrite `git_config()` using config-set API
Matthieu Moy writes: >>> Why is this needed? Are you now using key_value_info outside config.c? >>> Or is it a leftover from a previous experiment? >> >> Has this been resolved in the new round? > > Tanay explained in another subthread why this was needed. For callers > iterating over the string_list who want to get the file/line info, they > need to be able to cast the void * pointer to struct key_value_info *. For callers that want to see all the multi-values, it would be preferrable for the iterator to pass the filename and the linenumber to the callback function, instead of exposing its implementation detail as a single string list and telling them to pick it apart, no? Not a very convincing argument, but OK for now in the sense that we can fix it later if we wanted to before it gets too late. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/8] add line number and file name info to `config_set`
Junio C Hamano writes: > Matthieu Moy writes: > >> Ramsay Jones writes: >> ... diff --git a/cache.h b/cache.h ... +struct key_value_info { + const char *filename; + int linenr; +}; + >>> >>> I haven't checked, but does this series now include a user for >>> this struct outside of config.c? If not, then I think it would >>> be better to leave the declaration in config.c until it is needed. >>> (To make it easier to see if it is necessary in the context of the >>> patch which will make use of it). >> >> I disagree: this patch series is essentially about introducing a new >> API, and this struct declaration is part of the API. > > Hmm, is it? How would the customer of the API use it? die_config > and friends may internally use the information recorded using the > structure, but I had an impression that it is an implementation > detail that does not need to be exposed to the customers of the API. > Am I mistaken? It does if you want to provide error message while iterating over the string_list. Not the common case, but shouldn't be forbidden either. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] git_config callers rewritten with the new config-set API
Tanay Abhra writes: > 11 files changed, 114 insertions(+), 250 deletions(-) Nice reduction. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/8] add line number and file name info to `config_set`
Matthieu Moy writes: > Ramsay Jones writes: > ... >>> diff --git a/cache.h b/cache.h >>> ... >>> +struct key_value_info { >>> + const char *filename; >>> + int linenr; >>> +}; >>> + >> >> I haven't checked, but does this series now include a user for >> this struct outside of config.c? If not, then I think it would >> be better to leave the declaration in config.c until it is needed. >> (To make it easier to see if it is necessary in the context of the >> patch which will make use of it). > > I disagree: this patch series is essentially about introducing a new > API, and this struct declaration is part of the API. Hmm, is it? How would the customer of the API use it? die_config and friends may internally use the information recorded using the structure, but I had an impression that it is an implementation detail that does not need to be exposed to the customers of the API. Am I mistaken? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/8] add line number and file name info to `config_set`
Ramsay Jones writes: > On 07/08/14 12:59, Tanay Abhra wrote: >> Store file name and line number for each key-value pair in the cache >> during parsing of the configuration files. >> >> Signed-off-by: Tanay Abhra >> --- >> cache.h | 5 + >> config.c | 16 ++-- >> 2 files changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/cache.h b/cache.h >> index 7292aef..0b1bdfd 100644 >> --- a/cache.h >> +++ b/cache.h >> @@ -1383,6 +1383,11 @@ extern int git_config_get_bool_or_int(const char >> *key, int *is_bool, int *dest); >> extern int git_config_get_maybe_bool(const char *key, int *dest); >> extern int git_config_get_pathname(const char *key, const char **dest); >> >> +struct key_value_info { >> +const char *filename; >> +int linenr; >> +}; >> + > > I haven't checked, but does this series now include a user for > this struct outside of config.c? If not, then I think it would > be better to leave the declaration in config.c until it is needed. > (To make it easier to see if it is necessary in the context of the > patch which will make use of it). I disagree: this patch series is essentially about introducing a new API, and this struct declaration is part of the API. It seemed strange to me to see the code movement in the patch from two versions of the series, but the patch itself does not move the code, it just adds new code directly where it belongs. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/8] Rewrite `git_config()` using config-set API
Junio C Hamano writes: > Matthieu Moy writes: > >> Tanay Abhra writes: >> >>> --- a/cache.h >>> +++ b/cache.h >>> @@ -1406,8 +1406,14 @@ extern int git_config_get_bool(const char *key, int >>> *dest); >> [...] >>> +struct key_value_info { >>> + const char *filename; >>> + int linenr; >>> +}; >> [...] >>> diff --git a/config.c b/config.c >>> index cf9124f..427850a 100644 >>> --- a/config.c >>> +++ b/config.c >>> @@ -1224,11 +1224,6 @@ int git_config_with_options(config_fn_t fn, void >>> *data, >>> return ret; >>> } >>> >>> -struct key_value_info { >>> - const char *filename; >>> - int linenr; >>> -}; >>> - >> >> Why is this needed? Are you now using key_value_info outside config.c? >> Or is it a leftover from a previous experiment? > > Has this been resolved in the new round? Tanay explained in another subthread why this was needed. For callers iterating over the string_list who want to get the file/line info, they need to be able to cast the void * pointer to struct key_value_info *. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/8] add line number and file name info to `config_set`
On 07/08/14 12:59, Tanay Abhra wrote: > Store file name and line number for each key-value pair in the cache > during parsing of the configuration files. > > Signed-off-by: Tanay Abhra > --- > cache.h | 5 + > config.c | 16 ++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/cache.h b/cache.h > index 7292aef..0b1bdfd 100644 > --- a/cache.h > +++ b/cache.h > @@ -1383,6 +1383,11 @@ extern int git_config_get_bool_or_int(const char *key, > int *is_bool, int *dest); > extern int git_config_get_maybe_bool(const char *key, int *dest); > extern int git_config_get_pathname(const char *key, const char **dest); > > +struct key_value_info { > + const char *filename; > + int linenr; > +}; > + I haven't checked, but does this series now include a user for this struct outside of config.c? If not, then I think it would be better to leave the declaration in config.c until it is needed. (To make it easier to see if it is necessary in the context of the patch which will make use of it). ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] builtin/log.c: fix minor memory leak
Jonathan Nieder writes: > Matthieu Moy wrote: > >> Signed-off-by: Matthieu Moy >> --- >> Valgrind confirms, one less unreachable block ;-). > > This belongs in the commit message. > > [...] >> --- a/builtin/log.c >> +++ b/builtin/log.c >> @@ -857,20 +857,21 @@ static void add_branch_description(struct strbuf *buf, >> const char *branch_name) >> { >> struct strbuf desc = STRBUF_INIT; >> if (!branch_name || !*branch_name) >> return; >> read_branch_desc(&desc, branch_name); >> if (desc.len) { >> strbuf_addch(buf, '\n'); >> strbuf_addbuf(buf, &desc); >> strbuf_addch(buf, '\n'); >> } >> +strbuf_release(&desc); > > This is an old one. The leak was introduced by v1.7.9-rc1~1^2~12 > (format-patch: use branch description in cover letter, 2011-09-21). > > I was a little scared to see a leak in 'git log' code, since most of > what log does involves looping over many commits. Luckily this one is > only used in make_cover_letter to create a cover letter describing the > single branch on the command line, making it is a small, one-time > leak. Exactly ;-). > > Less noise from static and dynamic analysis tools is still worthwhile, > so for what it's worth, > > Reviewed-by: Jonathan Nieder > > Thanks. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/8] Rewrite `git_config()` using config-set API
Matthieu Moy writes: > Tanay Abhra writes: > >> --- a/cache.h >> +++ b/cache.h >> @@ -1406,8 +1406,14 @@ extern int git_config_get_bool(const char *key, int >> *dest); > [...] >> +struct key_value_info { >> +const char *filename; >> +int linenr; >> +}; > [...] >> diff --git a/config.c b/config.c >> index cf9124f..427850a 100644 >> --- a/config.c >> +++ b/config.c >> @@ -1224,11 +1224,6 @@ int git_config_with_options(config_fn_t fn, void >> *data, >> return ret; >> } >> >> -struct key_value_info { >> -const char *filename; >> -int linenr; >> -}; >> - > > Why is this needed? Are you now using key_value_info outside config.c? > Or is it a leftover from a previous experiment? Has this been resolved in the new round? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 5/8] config: add `git_die_config()` to the config-set API
Tanay Abhra writes: > diff --git a/Documentation/technical/api-config.txt > b/Documentation/technical/api-config.txt > index 21f280c..0d8b99b 100644 > --- a/Documentation/technical/api-config.txt > +++ b/Documentation/technical/api-config.txt > @@ -155,6 +155,19 @@ as well as retrieval for the queried variable, including: > Similar to `git_config_get_string`, but expands `~` or `~user` into > the user's home directory when found at the beginning of the path. > > +`git_die_config(const char *key, const char *err, ...)`:: > + > + First prints the error message specified by the caller in `err` and then > + dies printing the line number and the file name of the highest priority > + value for the configuration variable `key`. Reviewed with a wider context, I notice that this entry alone lacks the return type. I am assuming that this is just an oversight, and adding 'void ' in front of the filename to match the next entry is simple enough. > +`void git_die_config_linenr(const char *key, const char *filename, int > linenr)`:: > + ... > +extern NORETURN void git_die_config(const char *key, const char *err, ...) > __attribute__((format(printf, 2, 3))); > ... > +NORETURN __attribute__((format(printf, 2, 3))) > +void git_die_config(const char *key, const char *err, ...) > +{ My first reaction was that it might make the compiler unhappy to declare that the "err" is a printf-like format string and then to allow some callers to pass NULL to the function. My build however does not seem to complain, so perhaps this is OK. > + const struct string_list *values; > + struct key_value_info *kv_info; > + > + if (err) { > + va_list params; > + va_start(params, err); > + vreportf("error: ", err, params); > + va_end(params); > + } > + values = git_config_get_value_multi(key); > + kv_info = values->items[values->nr - 1].util; > + git_die_config_linenr(key, kv_info->filename, kv_info->linenr); > } > > /* -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: possibly a spurious conflict in a three way merge
Hi, meanlo...@gmail.com wrote: > 2. commit test.txt to master: > line1 > line1 > > 3. branch and checkout branch1 > 4. make and commit the following change to branch1: > #line1 > #line2 > > 5. checkout master > 6. make and commit the following change to master: > line1 > #line2 > > 7. merge branch1, git sees a conflict: > <<< HEAD > line1 > === > #line1 > >>> branch1 > #line2 > > Why? Thanks for a clear example. On branch1 you made the following change: (a) modify lines 1 and 2 On master, you made a different change: (b) just modify line 2 The changes touch the same chunk of lines and don't produce the same result there. Git is being conservative and letting you know that the two branches didn't agree about what line 1 should say. On the other hand, if you had a larger files and on branch1 made the following change: (a) modify lines 1 and 101 and on master, you made a different change: (b) just modify line 101 then the changes are touching different parts of the code and are merged independently. The result would be a clean merge where lines 1 and 101 are both modified. Git's conservatism can be helpful when working with code, where adjacent lines are likely to be affecting a single behavior and the conflict can help the operator to know to be extra careful. It makes less sense for line-oriented input where every line is independent. If you are often making changes to a line-oriented file, it may make sense to set up a custom merge driver to override git's merge behavior for that file. See 'Defining a custom merge driver' in gitattributes(5) for details about how that works. Hope that helps, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/11] git_config callers rewritten with the new config-set API
Tanay Abhra writes: > [v2]: git_die_config() messages changed. Diff between v1 and v2 is at the > bottom. I went through the series once more, and all the changes look good. v3 for PATCH 11/11 addresses my comment about the commit message in v2. Reviewed-by: Matthieu Moy -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] builtin/log.c: fix minor memory leak
Matthieu Moy wrote: > Signed-off-by: Matthieu Moy > --- > Valgrind confirms, one less unreachable block ;-). This belongs in the commit message. [...] > --- a/builtin/log.c > +++ b/builtin/log.c > @@ -857,20 +857,21 @@ static void add_branch_description(struct strbuf *buf, > const char *branch_name) > { > struct strbuf desc = STRBUF_INIT; > if (!branch_name || !*branch_name) > return; > read_branch_desc(&desc, branch_name); > if (desc.len) { > strbuf_addch(buf, '\n'); > strbuf_addbuf(buf, &desc); > strbuf_addch(buf, '\n'); > } > + strbuf_release(&desc); This is an old one. The leak was introduced by v1.7.9-rc1~1^2~12 (format-patch: use branch description in cover letter, 2011-09-21). I was a little scared to see a leak in 'git log' code, since most of what log does involves looping over many commits. Luckily this one is only used in make_cover_letter to create a cover letter describing the single branch on the command line, making it is a small, one-time leak. Less noise from static and dynamic analysis tools is still worthwhile, so for what it's worth, Reviewed-by: Jonathan Nieder Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 11/11] branch.c: replace `git_config()` with `git_config_get_string()
Use `git_config_get_string()` instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. While we are at it, return -1 if we find no value for the queried variable. Original code returned 0 for all cases, which was checked by `add_branch_desc()` in fmt-merge-msg.c resulting in addition of a spurious newline to the `out` strbuf. Now, the newline addition is skipped as -1 is returned to the caller if no value is found. Signed-off-by: Tanay Abhra --- v3: Changed the commit message to a more appropriate one. branch.c | 27 +++ 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/branch.c b/branch.c index 735767d..df6b120 100644 --- a/branch.c +++ b/branch.c @@ -140,30 +140,17 @@ static int setup_tracking(const char *new_ref, const char *orig_ref, return 0; } -struct branch_desc_cb { - const char *config_name; - const char *value; -}; - -static int read_branch_desc_cb(const char *var, const char *value, void *cb) -{ - struct branch_desc_cb *desc = cb; - if (strcmp(desc->config_name, var)) - return 0; - free((char *)desc->value); - return git_config_string(&desc->value, var, value); -} - int read_branch_desc(struct strbuf *buf, const char *branch_name) { - struct branch_desc_cb cb; + char *v = NULL; struct strbuf name = STRBUF_INIT; strbuf_addf(&name, "branch.%s.description", branch_name); - cb.config_name = name.buf; - cb.value = NULL; - git_config(read_branch_desc_cb, &cb); - if (cb.value) - strbuf_addstr(buf, cb.value); + if (git_config_get_string(name.buf, &v)) { + strbuf_release(&name); + return -1; + } + strbuf_addstr(buf, v); + free(v); strbuf_release(&name); return 0; } -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "Branch objects" (was: Re: cherry picking and merge)
Nico Williams wrote: > On Thu, Aug 07, 2014 at 05:42:34PM +0100, Tony Finch wrote: > > > > The problem is that the production branch gets copied around: pushed to > > the repo server, pulled by other team members, etc. Forced pushes > > are accident-prone, as is resetting a rebased branch after a pull. > > When I rebase and I need the old HEAD around I do something like this: > [...] > Either way I retain the old HEAD with some name. Hmm, yes, I can see that would work. However my previous workflow was rather branch-heavy and I found the accumulation of names annoying. I have not yet had enough usage out of git-repub to see if it goes too far in the direction of lack-of-names. A big omission is no opportunity to edit its commit messages. Tony. -- f.anthony.n.finchhttp://dotat.at/ Rockall: Southwesterly becoming cyclonic in north, 5 to 7. Moderate or rough. Rain or showers. Moderate or good. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "Branch objects" (was: Re: cherry picking and merge)
On Thu, Aug 07, 2014 at 05:42:34PM +0100, Tony Finch wrote: > Nico Williams wrote: > > On Thu, Aug 07, 2014 at 12:38:48PM +0100, Tony Finch wrote: > > > But [a rebasing workflow] is inconvenient for deploying the patched > > > version to production (which is the point of developing the fixes) - I > > > want a fast-forwarding branch for that. > > > > I'm not sure I follow this. You deploy what you build, and you build > > the HEAD of the production branch, whatever that is. If it gets > > rebased, so it it does. > > The problem is that the production branch gets copied around: pushed to > the repo server, pulled by other team members, etc. Forced pushes > are accident-prone, as is resetting a rebased branch after a pull. When I rebase and I need the old HEAD around I do something like this: $ git checkout $branch_to_rebase $ ver=${branch_to_rebase##*-} $ git checkout -b ${branch_to_rebase%-${ver}}-$((ver+1)) $ git rebase ... or like this: $ git checkout $branch_to_rebase $ git branch ${branch_to_rebase}-$(date +%Y-%m-%d) $ git rebase ... Either way I retain the old HEAD with some name. This requires discipline, so scripting it is useful. But if you want discipline then you want git to know that "for this branch, don't prune/gc old HEADs orphaned after rebases" and "push the rebase history for this branch". > > > https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git > > > > Yeah, that's useful. > > Glad you think so :-) Thank you. Nico -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] builtin/log.c: fix minor memory leak
Signed-off-by: Matthieu Moy --- Valgrind confirms, one less unreachable block ;-). builtin/log.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/log.c b/builtin/log.c index 4389722..e4d8122 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -857,20 +857,21 @@ static void add_branch_description(struct strbuf *buf, const char *branch_name) { struct strbuf desc = STRBUF_INIT; if (!branch_name || !*branch_name) return; read_branch_desc(&desc, branch_name); if (desc.len) { strbuf_addch(buf, '\n'); strbuf_addbuf(buf, &desc); strbuf_addch(buf, '\n'); } + strbuf_release(&desc); } static char *find_branch_name(struct rev_info *rev) { int i, positive = -1; unsigned char branch_sha1[20]; const unsigned char *tip_sha1; const char *ref, *v; char *full_ref, *branch = NULL; -- 2.0.2.737.gfb43bde -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/11] branch.c: replace `git_config()` with `git_config_get_string()
Tanay Abhra writes: > Use `git_config_get_string()` instead of `git_config()` to take advantage of > the config-set API which provides a cleaner control flow. > > Signed-off-by: Tanay Abhra > --- > branch.c | 27 +++ > 1 file changed, 7 insertions(+), 20 deletions(-) > > diff --git a/branch.c b/branch.c > index 735767d..df6b120 100644 > --- a/branch.c > +++ b/branch.c > @@ -140,30 +140,17 @@ static int setup_tracking(const char *new_ref, const > char *orig_ref, > return 0; > } > > -struct branch_desc_cb { > - const char *config_name; > - const char *value; > -}; > - > -static int read_branch_desc_cb(const char *var, const char *value, void *cb) > -{ > - struct branch_desc_cb *desc = cb; > - if (strcmp(desc->config_name, var)) > - return 0; > - free((char *)desc->value); > - return git_config_string(&desc->value, var, value); > -} > - > int read_branch_desc(struct strbuf *buf, const char *branch_name) > { > - struct branch_desc_cb cb; > + char *v = NULL; > struct strbuf name = STRBUF_INIT; > strbuf_addf(&name, "branch.%s.description", branch_name); > - cb.config_name = name.buf; > - cb.value = NULL; > - git_config(read_branch_desc_cb, &cb); > - if (cb.value) > - strbuf_addstr(buf, cb.value); > + if (git_config_get_string(name.buf, &v)) { > + strbuf_release(&name); > + return -1; > + } > + strbuf_addstr(buf, v); > + free(v); There's a behavior change here, but I think it is the right thing to do. It lacks a proper commit message though: As a reminder, your patch "change `git_config()` return value to void" in the other series did: --- a/branch.c +++ b/branch.c @@ -161,10 +161,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name) strbuf_addf(&name, "branch.%s.description", branch_name); cb.config_name = name.buf; cb.value = NULL; -if (git_config(read_branch_desc_cb, &cb) < 0) { -strbuf_release(&name); -return -1; -} +git_config(read_branch_desc_cb, &cb); if (cb.value) strbuf_addstr(buf, cb.value); strbuf_release(&name); So, before it, read_branch_desc() was returning -1 iff git_config() failed, which essentially never happened. Now, you're retoring a similar "if", but you strbuf_release and return -1 if no value is found for the variable. There are 3 callers of read_branch_desc: builtin/branch.c: read_branch_desc(&buf, branch_name); builtin/fmt-merge-msg.c:if (!read_branch_desc(&desc, name)) { builtin/log.c: read_branch_desc(&desc, branch_name); Only the one in fmt-merge-msg.c uses the return value: static void add_branch_desc(struct strbuf *out, const char *name) { struct strbuf desc = STRBUF_INIT; if (!read_branch_desc(&desc, name)) { const char *bp = desc.buf; while (*bp) { /* (1) */ const char *ep = strchrnul(bp, '\n'); if (*ep) ep++; strbuf_addf(out, " : %.*s", (int)(ep - bp), bp); bp = ep; } if (out->buf[out->len - 1] != '\n') /* (2) */ strbuf_addch(out, '\n'); } strbuf_release(&desc); } the (1) part is a no-op if no value is found, but the old code was still adding a \n in the (2) part, even when no value was found. So, the new code is better than the old one, but your patch does a bit more than the commit message claims. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "Branch objects" (was: Re: cherry picking and merge)
Nico Williams wrote: > On Thu, Aug 07, 2014 at 12:38:48PM +0100, Tony Finch wrote: > > > But [a rebasing workflow] is inconvenient for deploying the patched > > version to production (which is the point of developing the fixes) - I > > want a fast-forwarding branch for that. > > I'm not sure I follow this. You deploy what you build, and you build > the HEAD of the production branch, whatever that is. If it gets > rebased, so it it does. The problem is that the production branch gets copied around: pushed to the repo server, pulled by other team members, etc. Forced pushes are accident-prone, as is resetting a rebased branch after a pull. > > So I have a small tool which maintains a publication branch which > > tracks the head of a rebasing branch. It's reasonably satisfactory so > > far... > > > > https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git > > Yeah, that's useful. Glad you think so :-) Tony. -- f.anthony.n.finchhttp://dotat.at/ Thames: Northeast veering southeast 4 or 5, occasionally 6. Slight, becoming slight or moderate. Fair then rain or thundery showers. Good, becoming moderate or poor for a time. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] various contrib: Fix links in man pages
Stefan Beller writes: > Inspired by 2147fa7e (2014-07-31 git-push: fix link in man page), > I grepped through the whole tree searching for 'gitlink:' occurrences. > > Signed-off-by: Stefan Beller > --- Thanks; I did the same earlier but only for Documentation/ which was not very useful X-<. > contrib/convert-objects/git-convert-objects.txt | 2 +- > contrib/examples/git-svnimport.txt | 2 +- > contrib/gitview/gitview.txt | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/contrib/convert-objects/git-convert-objects.txt > b/contrib/convert-objects/git-convert-objects.txt > index 0565d83..f871880 100644 > --- a/contrib/convert-objects/git-convert-objects.txt > +++ b/contrib/convert-objects/git-convert-objects.txt > @@ -26,4 +26,4 @@ Documentation by David Greaves, Junio C Hamano and the > git-list > GIT > --- > -Part of the gitlink:git[7] suite > +Part of the linkgit:git[7] suite > diff --git a/contrib/examples/git-svnimport.txt > b/contrib/examples/git-svnimport.txt > index 3bb871e..3f0a9c3 100644 > --- a/contrib/examples/git-svnimport.txt > +++ b/contrib/examples/git-svnimport.txt > @@ -176,4 +176,4 @@ Documentation by Matthias Urlichs . > > GIT > --- > -Part of the gitlink:git[7] suite > +Part of the linkgit:git[7] suite > diff --git a/contrib/gitview/gitview.txt b/contrib/gitview/gitview.txt > index 9e12f97..7b5f900 100644 > --- a/contrib/gitview/gitview.txt > +++ b/contrib/gitview/gitview.txt > @@ -28,7 +28,7 @@ OPTIONS > > :: > > - All the valid option for gitlink:git-rev-list[1]. > + All the valid option for linkgit:git-rev-list[1]. > > Key Bindings > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#757297: 'git status' output is confusing after 'git add -N'
Duy Nguyen writes: > On Thu, Aug 7, 2014 at 7:34 AM, Jonathan Nieder wrote: >> Package: git >> Version: 1:2.0.0-1 >> Tags: upstream >> >> $ git init foo >> Initialized empty Git repository in /tmp/t/foo/.git/ >> $ cd foo >> $ echo hi >README >> $ git add -N README >> $ git status >> On branch master >> >> Initial commit >> >> Changes to be committed: >> (use "git rm --cached ..." to unstage) >> >> new file: README >> >> Changes not staged for commit: >> (use "git add ..." to update what will be committed) >> (use "git checkout -- ..." to discard changes in working directory) >> >> modified: README >> >> If I then run "git commit", it does not actually commit the addition >> of the README file. > > We used to reject such a commit operation before 3f6d56d (commit: > ignore intent-to-add entries instead of refusing - 2012-02-07) so it > was harder to misunderstand this case. > >> It would be clearer to have a separate section,like so: >> >> Tracked files not to be committed: >> (use "git rm --cached ..." to stop tracking) >> >>new file: README >> > > Or make the "Changes not staged for commit" part say "new file: > README" ("modified" is implied) Yeah, after reading the justification in the quoted commit, I agree that it is status that is at fault in the above; "new file: README" is part of "Changes not staged for commit" in this case (it is told to the index, but the user never said it is "for commit" yet, which is the whole point of "-N"), so instead of adding a new section, I agree that it should be classified as "new file" not "modified" there. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
possibly a spurious conflict in a three way merge
git 2.0.4 on ubuntu 14.04 64 1. new repo 2. commit test.txt to master: line1 line1 3. branch and checkout branch1 4. make and commit the following change to branch1: #line1 #line2 5. checkout master 6. make and commit the following change to master: line1 #line2 7. merge branch1, git sees a conflict: <<< HEAD line1 === #line1 >>> branch1 #line2 Why? The first line changed in branch1 but not in master so a 3 way merge should take branch1 changes. Beyond Compare ( used as a mergetool) does not flag any conflicts. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/11] git_config callers rewritten with the new config-set API
[v2]: git_die_config() messages changed. Diff between v1 and v2 is at the bottom. The ta/config-set API is more or less solidified. This series builds on the top of 4c715ebb in pu (ta/config-set). On top of it, it also requires series [1] (Rewrite `git_config()` using config-set API) for proper error checking. This series is the first batch of patches which rewrites the existing callers using a non-callback approach. This series aims to, * rewrite the existing callers, as you can see from the diff stat the bew API provides a much concise and clear control flow. * stress test the new API, see if any corner cases or deficiencies arise or not. The series passes all the tests, only thing to watch is that the config variables that have been rewritten are single valued only. Though I have tried my best to ascertain it, still mistakes may arise. [1]: http://thread.gmane.org/gmane.comp.version-control.git/254633/ Tanay Abhra (11): daemon.c: replace `git_config()` with `git_config_get_bool()` family http-backend.c: replace `git_config()` with `git_config_get_bool()` family read-cache.c: replace `git_config()` with `git_config_get_*()` family archive.c: replace `git_config()` with `git_config_get_bool()` family fetchpack.c: replace `git_config()` with `git_config_get_*()` family rerere.c: replace `git_config()` with `git_config_get_*()` family builtin/gc.c: replace `git_config()` with `git_config_get_*()` family pager.c: replace `git_config()` with `git_config_get_value()` imap-send.c: replace `git_config()` with `git_config_get_*()` family alias.c: replace `git_config()` with `git_config_get_string()` branch.c: replace `git_config()` with `git_config_get_string() alias.c| 25 ++-- archive.c | 12 +++- branch.c | 27 +++--- builtin/gc.c | 51 - daemon.c | 26 - fetch-pack.c | 35 -- http-backend.c | 31 -- imap-send.c| 60 +- pager.c| 40 +-- read-cache.c | 14 +++--- rerere.c | 43 - 11 files changed, 114 insertions(+), 250 deletions(-) -- 1.9.0.GIT -- 8< -- diff --git a/builtin/gc.c b/builtin/gc.c index 4612ef5..5173657 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -76,8 +76,8 @@ static void gc_config(void) if (strcmp(prune_expire, "now")) { unsigned long now = approxidate("now"); if (approxidate(prune_expire) >= now) { - error(_("Invalid %s: '%s'"), "gc.pruneexpire", prune_expire); - git_die_config("gc.pruneexpire"); + git_die_config("gc.pruneexpire", _("Invalid gc.pruneexpire: '%s'"), + prune_expire); } } } diff --git a/daemon.c b/daemon.c index fb16664..6f78b61 100644 --- a/daemon.c +++ b/daemon.c @@ -342,7 +342,6 @@ static int run_service(const char *dir, struct daemon_service *service) git_config_get_bool(var.buf, &enabled); strbuf_release(&var); } - if (!enabled) { logerror("'%s': service not enabled for '%s'", service->name, path); diff --git a/imap-send.c b/imap-send.c index 586bdd8..618d75b 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1336,8 +1336,7 @@ static void git_imap_config(void) if (!git_config_get_value("imap.host", &val)) { if (!val) { - config_error_nonbool("imap.host"); - git_die_config("imap.host"); + git_die_config("imap.host", "Missing value for 'imap.host'"); } else { if (starts_with(val, "imap:")) val += 5; -- 8< -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/11] alias.c: replace `git_config()` with `git_config_get_string()`
Use `git_config_get_string()` instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- alias.c | 25 ++--- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/alias.c b/alias.c index 758c867..6aa164a 100644 --- a/alias.c +++ b/alias.c @@ -1,26 +1,13 @@ #include "cache.h" -static const char *alias_key; -static char *alias_val; - -static int alias_lookup_cb(const char *k, const char *v, void *cb) -{ - const char *name; - if (skip_prefix(k, "alias.", &name) && !strcmp(name, alias_key)) { - if (!v) - return config_error_nonbool(k); - alias_val = xstrdup(v); - return 0; - } - return 0; -} - char *alias_lookup(const char *alias) { - alias_key = alias; - alias_val = NULL; - git_config(alias_lookup_cb, NULL); - return alias_val; + char *v = NULL; + struct strbuf key = STRBUF_INIT; + strbuf_addf(&key, "alias.%s", alias); + git_config_get_string(key.buf, &v); + strbuf_release(&key); + return v; } #define SPLIT_CMDLINE_BAD_ENDING 1 -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 01/11] daemon.c: replace `git_config()` with `git_config_get_bool()` family
Use `git_config_get_bool()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- daemon.c | 26 -- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/daemon.c b/daemon.c index e6b51ed..6f78b61 100644 --- a/daemon.c +++ b/daemon.c @@ -230,23 +230,6 @@ struct daemon_service { int overridable; }; -static struct daemon_service *service_looking_at; -static int service_enabled; - -static int git_daemon_config(const char *var, const char *value, void *cb) -{ - const char *service; - - if (skip_prefix(var, "daemon.", &service) && - !strcmp(service, service_looking_at->config_name)) { - service_enabled = git_config_bool(var, value); - return 0; - } - - /* we are not interested in parsing any other configuration here */ - return 0; -} - static int daemon_error(const char *dir, const char *msg) { if (!informative_errors) @@ -324,6 +307,7 @@ static int run_service(const char *dir, struct daemon_service *service) { const char *path; int enabled = service->enabled; + struct strbuf var = STRBUF_INIT; loginfo("Request %s for '%s'", service->name, dir); @@ -354,11 +338,9 @@ static int run_service(const char *dir, struct daemon_service *service) } if (service->overridable) { - service_looking_at = service; - service_enabled = -1; - git_config(git_daemon_config, NULL); - if (0 <= service_enabled) - enabled = service_enabled; + strbuf_addf(&var, "daemon.%s", service->config_name); + git_config_get_bool(var.buf, &enabled); + strbuf_release(&var); } if (!enabled) { logerror("'%s': service not enabled for '%s'", -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/11] builtin/gc.c: replace `git_config()` with `git_config_get_*()` family
Use `git_config_get_*()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- builtin/gc.c | 51 --- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index 8d219d8..ced1456 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -55,44 +55,33 @@ static void remove_pidfile_on_signal(int signo) raise(signo); } -static int gc_config(const char *var, const char *value, void *cb) +static void gc_config(void) { - if (!strcmp(var, "gc.packrefs")) { + const char *value; + + if (!git_config_get_value("gc.packrefs", &value)) { if (value && !strcmp(value, "notbare")) pack_refs = -1; else - pack_refs = git_config_bool(var, value); - return 0; - } - if (!strcmp(var, "gc.aggressivewindow")) { - aggressive_window = git_config_int(var, value); - return 0; - } - if (!strcmp(var, "gc.aggressivedepth")) { - aggressive_depth = git_config_int(var, value); - return 0; - } - if (!strcmp(var, "gc.auto")) { - gc_auto_threshold = git_config_int(var, value); - return 0; - } - if (!strcmp(var, "gc.autopacklimit")) { - gc_auto_pack_limit = git_config_int(var, value); - return 0; + pack_refs = git_config_bool("gc.packrefs", value); } - if (!strcmp(var, "gc.autodetach")) { - detach_auto = git_config_bool(var, value); - return 0; - } - if (!strcmp(var, "gc.pruneexpire")) { - if (value && strcmp(value, "now")) { + + git_config_get_int("gc.aggressivewindow", &aggressive_window); + git_config_get_int("gc.aggressivedepth", &aggressive_depth); + git_config_get_int("gc.auto", &gc_auto_threshold); + git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit); + git_config_get_bool("gc.autodetach", &detach_auto); + + if (!git_config_get_string_const("gc.pruneexpire", &prune_expire)) { + if (strcmp(prune_expire, "now")) { unsigned long now = approxidate("now"); - if (approxidate(value) >= now) - return error(_("Invalid %s: '%s'"), var, value); + if (approxidate(prune_expire) >= now) { + git_die_config("gc.pruneexpire", _("Invalid gc.pruneexpire: '%s'"), + prune_expire); + } } - return git_config_string(&prune_expire, var, value); } - return git_default_config(var, value, cb); + git_config(git_default_config, NULL); } static int too_many_loose_objects(void) @@ -301,7 +290,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) argv_array_pushl(&prune, "prune", "--expire", NULL ); argv_array_pushl(&rerere, "rerere", "gc", NULL); - git_config(gc_config, NULL); + gc_config(); if (pack_refs < 0) pack_refs = !is_bare_repository(); -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/11] pager.c: replace `git_config()` with `git_config_get_value()`
Use `git_config_get_value()` instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- pager.c | 40 +--- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/pager.c b/pager.c index 8b5cbc5..b7eb7e7 100644 --- a/pager.c +++ b/pager.c @@ -6,12 +6,6 @@ #define DEFAULT_PAGER "less" #endif -struct pager_config { - const char *cmd; - int want; - char *value; -}; - /* * This is split up from the rest of git so that we can do * something different on Windows. @@ -155,30 +149,22 @@ int decimal_width(int number) return width; } -static int pager_command_config(const char *var, const char *value, void *data) +/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */ +int check_pager_config(const char *cmd) { - struct pager_config *c = data; - if (starts_with(var, "pager.") && !strcmp(var + 6, c->cmd)) { - int b = git_config_maybe_bool(var, value); + int want = -1; + struct strbuf key = STRBUF_INIT; + const char *value = NULL; + strbuf_addf(&key, "pager.%s", cmd); + if (!git_config_get_value(key.buf, &value)) { + int b = git_config_maybe_bool(key.buf, value); if (b >= 0) - c->want = b; + want = b; else { - c->want = 1; - c->value = xstrdup(value); + want = 1; + pager_program = xstrdup(value); } } - return 0; -} - -/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */ -int check_pager_config(const char *cmd) -{ - struct pager_config c; - c.cmd = cmd; - c.want = -1; - c.value = NULL; - git_config(pager_command_config, &c); - if (c.value) - pager_program = c.value; - return c.want; + strbuf_release(&key); + return want; } -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/11] read-cache.c: replace `git_config()` with `git_config_get_*()` family
Use `git_config_get_*()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Use an intermediate value, as `version` can not be used directly in git_config_get_int() due to incompatible type. Signed-off-by: Tanay Abhra --- read-cache.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/read-cache.c b/read-cache.c index 5d3c8bd..acb132d 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1238,24 +1238,16 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, #define INDEX_FORMAT_DEFAULT 3 -static int index_format_config(const char *var, const char *value, void *cb) -{ - unsigned int *version = cb; - if (!strcmp(var, "index.version")) { - *version = git_config_int(var, value); - return 0; - } - return 1; -} - static unsigned int get_index_format_default(void) { char *envversion = getenv("GIT_INDEX_VERSION"); char *endp; + int value; unsigned int version = INDEX_FORMAT_DEFAULT; if (!envversion) { - git_config(index_format_config, &version); + if (!git_config_get_int("index.version", &value)) + version = value; if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) { warning(_("index.version set, but the value is invalid.\n" "Using version %i"), INDEX_FORMAT_DEFAULT); -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/11] imap-send.c: replace `git_config()` with `git_config_get_*()` family
Use `git_config_get_*()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- imap-send.c | 60 ++-- 1 file changed, 26 insertions(+), 34 deletions(-) diff --git a/imap-send.c b/imap-send.c index 524fbab..618d75b 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1326,43 +1326,35 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs) static char *imap_folder; -static int git_imap_config(const char *key, const char *val, void *cb) +static void git_imap_config(void) { - if (!skip_prefix(key, "imap.", &key)) - return 0; + const char *val = NULL; + + git_config_get_bool("imap.sslverify", &server.ssl_verify); + git_config_get_bool("imap.preformattedhtml", &server.use_html); + git_config_get_string("imap.folder", &imap_folder); - /* check booleans first, and barf on others */ - if (!strcmp("sslverify", key)) - server.ssl_verify = git_config_bool(key, val); - else if (!strcmp("preformattedhtml", key)) - server.use_html = git_config_bool(key, val); - else if (!val) - return config_error_nonbool(key); - - if (!strcmp("folder", key)) { - imap_folder = xstrdup(val); - } else if (!strcmp("host", key)) { - if (starts_with(val, "imap:")) - val += 5; - else if (starts_with(val, "imaps:")) { - val += 6; - server.use_ssl = 1; + if (!git_config_get_value("imap.host", &val)) { + if (!val) { + git_die_config("imap.host", "Missing value for 'imap.host'"); + } else { + if (starts_with(val, "imap:")) + val += 5; + else if (starts_with(val, "imaps:")) { + val += 6; + server.use_ssl = 1; + } + if (starts_with(val, "//")) + val += 2; + server.host = xstrdup(val); } - if (starts_with(val, "//")) - val += 2; - server.host = xstrdup(val); - } else if (!strcmp("user", key)) - server.user = xstrdup(val); - else if (!strcmp("pass", key)) - server.pass = xstrdup(val); - else if (!strcmp("port", key)) - server.port = git_config_int(key, val); - else if (!strcmp("tunnel", key)) - server.tunnel = xstrdup(val); - else if (!strcmp("authmethod", key)) - server.auth_method = xstrdup(val); + } - return 0; + git_config_get_string("imap.user", &server.user); + git_config_get_string("imap.pass", &server.pass); + git_config_get_int("imap.port", &server.port); + git_config_get_string("imap.tunnel", &server.tunnel); + git_config_get_string("imap.authmethod", &server.auth_method); } int main(int argc, char **argv) @@ -1383,7 +1375,7 @@ int main(int argc, char **argv) usage(imap_send_usage); setup_git_directory_gently(&nongit_ok); - git_config(git_imap_config, NULL); + git_imap_config(); if (!server.port) server.port = server.use_ssl ? 993 : 143; -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 11/11] branch.c: replace `git_config()` with `git_config_get_string()
Use `git_config_get_string()` instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- branch.c | 27 +++ 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/branch.c b/branch.c index 735767d..df6b120 100644 --- a/branch.c +++ b/branch.c @@ -140,30 +140,17 @@ static int setup_tracking(const char *new_ref, const char *orig_ref, return 0; } -struct branch_desc_cb { - const char *config_name; - const char *value; -}; - -static int read_branch_desc_cb(const char *var, const char *value, void *cb) -{ - struct branch_desc_cb *desc = cb; - if (strcmp(desc->config_name, var)) - return 0; - free((char *)desc->value); - return git_config_string(&desc->value, var, value); -} - int read_branch_desc(struct strbuf *buf, const char *branch_name) { - struct branch_desc_cb cb; + char *v = NULL; struct strbuf name = STRBUF_INIT; strbuf_addf(&name, "branch.%s.description", branch_name); - cb.config_name = name.buf; - cb.value = NULL; - git_config(read_branch_desc_cb, &cb); - if (cb.value) - strbuf_addstr(buf, cb.value); + if (git_config_get_string(name.buf, &v)) { + strbuf_release(&name); + return -1; + } + strbuf_addstr(buf, v); + free(v); strbuf_release(&name); return 0; } -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/11] archive.c: replace `git_config()` with `git_config_get_bool()` family
Use `git_config_get_bool()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- archive.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/archive.c b/archive.c index 3fc0fb2..952a659 100644 --- a/archive.c +++ b/archive.c @@ -402,14 +402,6 @@ static int parse_archive_args(int argc, const char **argv, return argc; } -static int git_default_archive_config(const char *var, const char *value, - void *cb) -{ - if (!strcmp(var, "uploadarchive.allowunreachable")) - remote_allow_unreachable = git_config_bool(var, value); - return git_default_config(var, value, cb); -} - int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint, int remote) { @@ -420,7 +412,9 @@ int write_archive(int argc, const char **argv, const char *prefix, if (setup_prefix && prefix == NULL) prefix = setup_git_directory_gently(&nongit); - git_config(git_default_archive_config, NULL); + git_config_get_bool("uploadarchive.allowunreachable", &remote_allow_unreachable); + git_config(git_default_config, NULL); + init_tar_archiver(); init_zip_archiver(); -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/11] http-backend.c: replace `git_config()` with `git_config_get_bool()` family
Use `git_config_get_bool()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- http-backend.c | 31 --- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/http-backend.c b/http-backend.c index 80790bb..106ca6b 100644 --- a/http-backend.c +++ b/http-backend.c @@ -219,29 +219,22 @@ static void get_idx_file(char *name) send_local_file("application/x-git-packed-objects-toc", name); } -static int http_config(const char *var, const char *value, void *cb) +static void http_config(void) { - const char *p; + int i, value = 0; + struct strbuf var = STRBUF_INIT; - if (!strcmp(var, "http.getanyfile")) { - getanyfile = git_config_bool(var, value); - return 0; - } + git_config_get_bool("http.getanyfile", &getanyfile); - if (skip_prefix(var, "http.", &p)) { - int i; - - for (i = 0; i < ARRAY_SIZE(rpc_service); i++) { - struct rpc_service *svc = &rpc_service[i]; - if (!strcmp(p, svc->config_name)) { - svc->enabled = git_config_bool(var, value); - return 0; - } - } + for (i = 0; i < ARRAY_SIZE(rpc_service); i++) { + struct rpc_service *svc = &rpc_service[i]; + strbuf_addf(&var, "http.%s", svc->config_name); + if (!git_config_get_bool(var.buf, &value)) + svc->enabled = value; + strbuf_reset(&var); } - /* we are not interested in parsing any other configuration here */ - return 0; + strbuf_release(&var); } static struct rpc_service *select_service(const char *name) @@ -627,7 +620,7 @@ int main(int argc, char **argv) access("git-daemon-export-ok", F_OK) ) not_found("Repository not exported: '%s'", dir); - git_config(http_config, NULL); + http_config(); cmd->imp(cmd_arg); return 0; } -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/11] rerere.c: replace `git_config()` with `git_config_get_*()` family
Use `git_config_get_*()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- rerere.c | 43 --- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/rerere.c b/rerere.c index d84b495..20b18ad 100644 --- a/rerere.c +++ b/rerere.c @@ -573,15 +573,11 @@ static int do_plain_rerere(struct string_list *rr, int fd) return write_rr(rr, fd); } -static int git_rerere_config(const char *var, const char *value, void *cb) +static void git_rerere_config(void) { - if (!strcmp(var, "rerere.enabled")) - rerere_enabled = git_config_bool(var, value); - else if (!strcmp(var, "rerere.autoupdate")) - rerere_autoupdate = git_config_bool(var, value); - else - return git_default_config(var, value, cb); - return 0; + git_config_get_bool("rerere.enabled", &rerere_enabled); + git_config_get_bool("rerere.autoupdate", &rerere_autoupdate); + git_config(git_default_config, NULL); } static int is_rerere_enabled(void) @@ -606,7 +602,7 @@ int setup_rerere(struct string_list *merge_rr, int flags) { int fd; - git_config(git_rerere_config, NULL); + git_rerere_config(); if (!is_rerere_enabled()) return -1; @@ -699,24 +695,6 @@ static void unlink_rr_item(const char *name) rmdir(git_path("rr-cache/%s", name)); } -struct rerere_gc_config_cb { - int cutoff_noresolve; - int cutoff_resolve; -}; - -static int git_rerere_gc_config(const char *var, const char *value, void *cb) -{ - struct rerere_gc_config_cb *cf = cb; - - if (!strcmp(var, "gc.rerereresolved")) - cf->cutoff_resolve = git_config_int(var, value); - else if (!strcmp(var, "gc.rerereunresolved")) - cf->cutoff_noresolve = git_config_int(var, value); - else - return git_default_config(var, value, cb); - return 0; -} - void rerere_gc(struct string_list *rr) { struct string_list to_remove = STRING_LIST_INIT_DUP; @@ -724,9 +702,12 @@ void rerere_gc(struct string_list *rr) struct dirent *e; int i, cutoff; time_t now = time(NULL), then; - struct rerere_gc_config_cb cf = { 15, 60 }; + int cutoff_noresolve = 15; + int cutoff_resolve = 60; - git_config(git_rerere_gc_config, &cf); + git_config_get_int("gc.rerereresolved", &cutoff_resolve); + git_config_get_int("gc.rerereunresolved", &cutoff_noresolve); + git_config(git_default_config, NULL); dir = opendir(git_path("rr-cache")); if (!dir) die_errno("unable to open rr-cache directory"); @@ -736,12 +717,12 @@ void rerere_gc(struct string_list *rr) then = rerere_last_used_at(e->d_name); if (then) { - cutoff = cf.cutoff_resolve; + cutoff = cutoff_resolve; } else { then = rerere_created_at(e->d_name); if (!then) continue; - cutoff = cf.cutoff_noresolve; + cutoff = cutoff_noresolve; } if (then < now - cutoff * 86400) string_list_append(&to_remove, e->d_name); -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/11] fetchpack.c: replace `git_config()` with `git_config_get_*()` family
Use `git_config_get_*()` family instead of `git_config()` to take advantage of the config-set API which provides a cleaner control flow. Signed-off-by: Tanay Abhra --- fetch-pack.c | 35 --- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/fetch-pack.c b/fetch-pack.c index b8a58fa..a13e9db 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -869,34 +869,15 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args, return ref; } -static int fetch_pack_config(const char *var, const char *value, void *cb) +static void fetch_pack_config(void) { - if (strcmp(var, "fetch.unpacklimit") == 0) { - fetch_unpack_limit = git_config_int(var, value); - return 0; - } - - if (strcmp(var, "transfer.unpacklimit") == 0) { - transfer_unpack_limit = git_config_int(var, value); - return 0; - } - - if (strcmp(var, "repack.usedeltabaseoffset") == 0) { - prefer_ofs_delta = git_config_bool(var, value); - return 0; - } - - if (!strcmp(var, "fetch.fsckobjects")) { - fetch_fsck_objects = git_config_bool(var, value); - return 0; - } - - if (!strcmp(var, "transfer.fsckobjects")) { - transfer_fsck_objects = git_config_bool(var, value); - return 0; - } + git_config_get_int("fetch.unpacklimit", &fetch_unpack_limit); + git_config_get_int("transfer.unpacklimit", &transfer_unpack_limit); + git_config_get_bool("repack.usedeltabaseoffset", &prefer_ofs_delta); + git_config_get_bool("fetch.fsckobjects", &fetch_fsck_objects); + git_config_get_bool("transfer.fsckobjects", &transfer_fsck_objects); - return git_default_config(var, value, cb); + git_config(git_default_config, NULL); } static void fetch_pack_setup(void) @@ -904,7 +885,7 @@ static void fetch_pack_setup(void) static int did_setup; if (did_setup) return; - git_config(fetch_pack_config, NULL); + fetch_pack_config(); if (0 <= transfer_unpack_limit) unpack_limit = transfer_unpack_limit; else if (0 <= fetch_unpack_limit) -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Subtree with submodule inside?
On Wed, Aug 06, 2014 at 04:51:52PM -0700, Jonathan Nieder wrote: > Junio C Hamano wrote: > > Jonathan Nieder writes: > > >> 2. Submodules aware of their superproject and of the parent's branches. > >> In other words, submodules would act as though under refs/ they > >> had a symlink > >> > >>parent -> ../../../refs > >> > >> So you could do > >> > >>git checkout --recurse-submodules master > >> > >>cd path/to/submodule > >>git checkout parent/heads/next > >> > >> This would avoid danger from "git gc" in submodules and would > >> get rid of most of the motivation for named branches in the > >> submodule, I'd think. > > > > Are you assuming that they share their object stores? > > No. The 'symlink' thing is a think-o. (When trying to explain the > idea I ended up oversimplifying and speaking nonsense.) > > What I wanted to say is that parent/heads/next would be a way to > refer from the submodule to the same commit as > > refs/heads/next:path/to/submodule > > refers to in the parent. I like this idea. It could solve many issues and help in many cases I think. Since we are currently quite busy with other things I took the liberty of adding an ideas section in Jens submodule wiki[1]. This way we do not forget about it and/or can refer others to it more easily. I would appreciate if someone could have a look whether I described the idea clearly enough. Cheers Heiko [1] https://github.com/jlehmann/git-submod-enhancements/wiki#dynamic-superproject-refs-in-submodules -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "Branch objects" (was: Re: cherry picking and merge)
On Thu, Aug 07, 2014 at 12:38:48PM +0100, Tony Finch wrote: > I have been fiddling around in this area. > > What I want to be able to do is develop fixes for open source code > that I run, and get those fixes upstream. This means I need a rebasing > workflow, to keep the patches up-to-date and to deal with code review > feedback. Right. > But this is inconvenient for deploying the patched version to > production (which is the point of developing the fixes) - I want a I'm not sure I follow this. You deploy what you build, and you build the HEAD of the production branch, whatever that is. If it gets rebased, so it it does. > fast-forwarding branch for that. And it would be nice to be able to > share the history of the patch series, so others can see what changed > between revisions more easily. But yes, it's nice to have a history of all the rebases. For example: so you can show the work you've done (rebasing to please an upstream is work). The reflog does this, of course, but you can't push it. Of course, my conception of branch object wouldn't push rebase history to an upstream that doesn't want it, but you could push it to repos that do. > So I have a small tool which maintains a publication branch which > tracks the head of a rebasing branch. It's reasonably satisfactory so > far... > > https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git Yeah, that's useful. Nico -- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] various contrib: Fix links in man pages
Inspired by 2147fa7e (2014-07-31 git-push: fix link in man page), I grepped through the whole tree searching for 'gitlink:' occurrences. Signed-off-by: Stefan Beller --- contrib/convert-objects/git-convert-objects.txt | 2 +- contrib/examples/git-svnimport.txt | 2 +- contrib/gitview/gitview.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/convert-objects/git-convert-objects.txt b/contrib/convert-objects/git-convert-objects.txt index 0565d83..f871880 100644 --- a/contrib/convert-objects/git-convert-objects.txt +++ b/contrib/convert-objects/git-convert-objects.txt @@ -26,4 +26,4 @@ Documentation by David Greaves, Junio C Hamano and the git-list . GIT --- -Part of the gitlink:git[7] suite +Part of the linkgit:git[7] suite diff --git a/contrib/gitview/gitview.txt b/contrib/gitview/gitview.txt index 9e12f97..7b5f900 100644 --- a/contrib/gitview/gitview.txt +++ b/contrib/gitview/gitview.txt @@ -28,7 +28,7 @@ OPTIONS :: - All the valid option for gitlink:git-rev-list[1]. + All the valid option for linkgit:git-rev-list[1]. Key Bindings -- 2.1.0.rc0.52.gaa544bf -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Pluggable backends for refs,wip
On 08/05/2014 02:40 PM, Ronnie Sahlberg wrote: > Please see > https://github.com/rsahlberg/git/tree/backend-struct-db-2 > for an example of a pluggable backend for refs storage. > > This series contain changes to make it possible to add new backends > for handling/storage of refs and implements one new backend : > refs-be-be.c . > > This new backend offloads the actual refs handling to a small database > daemon with which ita talks via a very simple rpc protocol. That > daemon in turn then connects to the datastore and read/writes the > values to it. > [...] Ronnie, This is awesome! Congratulations on your progress. I'm still on vacation and haven't yet looked at the code. I will be back next week and hope to find time to check it out, and also to do some more review of the code that you have already submitted to git core. Have you thought about how to test alternate reference backends? This will be very important to getting one or more of them accepted into git core (not to mention giving people confidence to actually *use* them!) It seems to me that a few steps are needed: * Each backend would need a suite of backend-aware tests that verify proper operation *within* the backend. These tests would mostly use low-level plumbing commands like update-refs to create/modify/delete references, and would be allowed to grub around in the filesystem, talk directly with the database, etc. to make sure that the commands have the correct effects. For example, for the traditional filesystem backend, these tests would be the ones to check that creating a reference causes a file to spring into existence under $GIT_DIR/refs. The tests for pack-refs, and all tests that care about the distinction between packed and loose refs, would become part of the backend-aware tests for the filesystem backend. All of the backend-aware tests should be run every time the test suite is run (provided, of course, that the correct prerequisites are available, and subject to being turned off manually). * The rest of the test suite has to be made backend-agnostic. For example, such tests should *not* be allowed to look under $GIT_DIR for the existence/absence of loose reference files [1] but would rather have to inquire about references via git commands. * It should be possible for the developer to choose easily which reference backend to use when running the agnostic part of the test suite. The chosen backend should be used to run *all* backend-agnostic tests. A database-backed backend might even want to be testable in two modes: one with the DB daemon running constantly, and one where the daemon is stopped and started between each pair of Git commands. So after the changes, a single run of the test suite should run the backend-aware tests for *all* known backends followed by the backend-agnostic tests for a single selected backend. Michael [1] When I was working on my quagga-reference spike [2] I found that a lot of the test suite uses knowledge about how references and reflogs are stored by the filesystem backend and just grabs at the files rather than accessing the references using git commands. It will take some work to clean this up. [2] http://thread.gmane.org/gmane.comp.version-control.git/243726 -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 0/8] Rewrite `git_config()` using config-set API
Tanay Abhra writes: > [Patch v9]: Changed the grep statements in patch 7/8 and 8/8. Good. I think it adresses all previous comments. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 8/8] add tests for `git_config_get_string_const()`
Add tests for `git_config_get_string_const()`, check whether it dies printing the line number and the file name if a NULL value is retrieved for the given key. Signed-off-by: Tanay Abhra --- t/t1308-config-set.sh | 10 ++ test-config.c | 10 ++ 2 files changed, 20 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 9cc678d..ea0bce2 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -119,6 +119,16 @@ test_expect_success 'find integer value for a key' ' check_config get_int lamb.chop 65 ' +test_expect_success 'find string value for a key' ' + check_config get_string case.baz hask && + check_config expect_code 1 get_string case.ba "Value not found for \"case.ba\"" +' + +test_expect_success 'check line error when NULL string is queried' ' + test_expect_code 128 test-config get_string case.foo 2>result && + test_i18ngrep "fatal: .*case\.foo.*\.git/config.*line 7" result +' + test_expect_success 'find integer if value is non parse-able' ' check_config expect_code 128 get_int lamb.head ' diff --git a/test-config.c b/test-config.c index 9dd1b22..6a77552 100644 --- a/test-config.c +++ b/test-config.c @@ -16,6 +16,8 @@ * * get_bool -> print bool value for the entered key or die * + * get_string -> print string value for the entered key or die + * * configset_get_value -> returns value with the highest priority for the entered key * from a config_set constructed from files entered as arguments. * @@ -84,6 +86,14 @@ int main(int argc, char **argv) printf("Value not found for \"%s\"\n", argv[2]); goto exit1; } + } else if (argc == 3 && !strcmp(argv[1], "get_string")) { + if (!git_config_get_string_const(argv[2], &v)) { + printf("%s\n", v); + goto exit0; + } else { + printf("Value not found for \"%s\"\n", argv[2]); + goto exit1; + } } else if (!strcmp(argv[1], "configset_get_value")) { for (i = 3; i < argc; i++) { int err; -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 6/8] rewrite git_config() to use the config-set API
Of all the functions in `git_config*()` family, `git_config()` has the most invocations in the whole code base. Each `git_config()` invocation causes config file rereads which can be avoided using the config-set API. Use the config-set API to rewrite `git_config()` to use the config caching layer to avoid config file rereads on each invocation during a git process lifetime. First invocation constructs the cache, and after that for each successive invocation, `git_config()` feeds values from the config cache instead of rereading the configuration files. Signed-off-by: Tanay Abhra --- cache.h | 24 +++ config.c| 51 + t/t4055-diff-context.sh | 2 +- 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/cache.h b/cache.h index 89a0d51..2693a37 100644 --- a/cache.h +++ b/cache.h @@ -8,6 +8,7 @@ #include "gettext.h" #include "convert.h" #include "trace.h" +#include "string-list.h" #include SHA1_HEADER #ifndef git_SHA_CTX @@ -1351,9 +1352,32 @@ extern int parse_config_key(const char *var, const char **subsection, int *subsection_len, const char **key); +struct config_set_element { + struct hashmap_entry ent; + char *key; + struct string_list value_list; +}; + +struct configset_list_item { + struct config_set_element *e; + int value_index; +}; + +/* + * the contents of the list are ordered according to their + * position in the config files and order of parsing the files. + * (i.e. key-value pair at the last position of .git/config will + * be at the last item of the list) + */ +struct configset_list { + struct configset_list_item *items; + unsigned int nr, alloc; +}; + struct config_set { struct hashmap config_hash; int hash_initialized; + struct configset_list list; }; extern void git_configset_init(struct config_set *cs); diff --git a/config.c b/config.c index 5ae9ab0..427850a 100644 --- a/config.c +++ b/config.c @@ -35,12 +35,6 @@ struct config_source { long (*do_ftell)(struct config_source *c); }; -struct config_set_element { - struct hashmap_entry ent; - char *key; - struct string_list value_list; -}; - static struct config_source *cf; static int zlib_compression_seen; @@ -1230,7 +1224,7 @@ int git_config_with_options(config_fn_t fn, void *data, return ret; } -void git_config(config_fn_t fn, void *data) +static void git_config_raw(config_fn_t fn, void *data) { if (git_config_with_options(fn, data, NULL, 1) < 0) /* @@ -1247,6 +1241,33 @@ void git_config(config_fn_t fn, void *data) die(_("unknown error occured while reading the configuration files")); } +static void configset_iter(struct config_set *cs, config_fn_t fn, void *data) +{ + int i, value_index; + struct string_list *values; + struct config_set_element *entry; + struct configset_list *list = &cs->list; + struct key_value_info *kv_info; + + for (i = 0; i < list->nr; i++) { + entry = list->items[i].e; + value_index = list->items[i].value_index; + values = &entry->value_list; + if (fn(entry->key, values->items[value_index].string, data) < 0) { + kv_info = values->items[value_index].util; + git_die_config_linenr(entry->key, kv_info->filename, kv_info->linenr); + } + } +} + +static void git_config_check_init(void); + +void git_config(config_fn_t fn, void *data) +{ + git_config_check_init(); + configset_iter(&the_config_set, fn, data); +} + static struct config_set_element *configset_find_element(struct config_set *cs, const char *key) { struct config_set_element k; @@ -1273,6 +1294,7 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha { struct config_set_element *e; struct string_list_item *si; + struct configset_list_item *l_item; struct key_value_info *kv_info = xmalloc(sizeof(*kv_info)); e = configset_find_element(cs, key); @@ -1288,6 +1310,12 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha hashmap_add(&cs->config_hash, e); } si = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL); + + ALLOC_GROW(cs->list.items, cs->list.nr + 1, cs->list.alloc); + l_item = &cs->list.items[cs->list.nr++]; + l_item->e = e; + l_item->value_index = e->value_list.nr - 1; + if (cf) { kv_info->filename = strintern(cf->name); kv_info->linenr = cf->linenr; @@ -1311,6 +1339,9 @@ void git_configset_init(struct config_set *cs) { hashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp, 0); cs->hash_initialized = 1;
[PATCH v9 7/8] add a test for semantic errors in config files
Semantic errors (for example, for alias.* variables NULL values are not allowed) in configuration files cause a die printing the line number and file name of the offending value. Add a test documenting that such errors cause a die printing the accurate line number and file name. Signed-off-by: Tanay Abhra --- t/t1308-config-set.sh | 11 +++ 1 file changed, 11 insertions(+) diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh index 7fdf840..9cc678d 100755 --- a/t/t1308-config-set.sh +++ b/t/t1308-config-set.sh @@ -197,4 +197,15 @@ test_expect_success 'proper error on error in custom config files' ' test_cmp expect actual ' +test_expect_success 'check line errors for malformed values' ' + mv .git/config .git/config.old && + test_when_finished "mv .git/config.old .git/config" && + cat >.git/config <<-\EOF && + [alias] + br + EOF + test_expect_code 128 git br 2>result && + test_i18ngrep "fatal: .*alias\.br.*\.git/config.*line 2" result +' + test_done -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 2/8] config.c: fix accuracy of line number in errors
From: Matthieu Moy If a callback returns a negative value to `git_config*()` family, they call `die()` while printing the line number and the file name. Currently the printed line number is off by one, thus printing the wrong line number. Make `linenr` point to the line we just parsed during the call to callback to get accurate line number in error messages. Commit-message-by: Tanay Abhra Signed-off-by: Tanay Abhra Signed-off-by: Matthieu Moy --- config.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 34940fd..bb4629e 100644 --- a/config.c +++ b/config.c @@ -244,6 +244,7 @@ static int get_next_char(void) cf->linenr++; if (c == EOF) { cf->eof = 1; + cf->linenr++; c = '\n'; } return c; @@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) { int c; char *value; + int ret; /* Get the full name */ for (;;) { @@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) if (!value) return -1; } - return fn(name->buf, value, data); + /* +* We already consumed the \n, but we need linenr to point to +* the line we just parsed during the call to fn to get +* accurate line number in error messages. +*/ + cf->linenr--; + ret = fn(name->buf, value, data); + cf->linenr++; + return ret; } static int get_extended_base_var(struct strbuf *name, int c) -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 4/8] change `git_config()` return value to void
Currently `git_config()` returns an integer signifying an error code. During rewrites of the function most of the code was shifted to `git_config_with_options()`. `git_config_with_options()` normally returns positive values if its `config_source` parameter is set as NULL, as most errors are fatal, and non-fatal potential errors are guarded by "if" statements that are entered only when no error is possible. Still a negative value can be returned in case of race condition between `access_or_die()` & `git_config_from_file()`. Also, all callers of `git_config()` ignore the return value except for one case in branch.c. Change `git_config()` return value to void and make it die if it receives a negative value from `git_config_with_options()`. Original-patch-by: Matthieu Moy Signed-off-by: Tanay Abhra --- branch.c | 5 + cache.h | 2 +- config.c | 16 ++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/branch.c b/branch.c index 46e8aa8..735767d 100644 --- a/branch.c +++ b/branch.c @@ -161,10 +161,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name) strbuf_addf(&name, "branch.%s.description", branch_name); cb.config_name = name.buf; cb.value = NULL; - if (git_config(read_branch_desc_cb, &cb) < 0) { - strbuf_release(&name); - return -1; - } + git_config(read_branch_desc_cb, &cb); if (cb.value) strbuf_addstr(buf, cb.value); strbuf_release(&name); diff --git a/cache.h b/cache.h index 0b1bdfd..f11ce41 100644 --- a/cache.h +++ b/cache.h @@ -1294,7 +1294,7 @@ extern int git_config_from_buf(config_fn_t fn, const char *name, const char *buf, size_t len, void *data); extern void git_config_push_parameter(const char *text); extern int git_config_from_parameters(config_fn_t fn, void *data); -extern int git_config(config_fn_t fn, void *); +extern void git_config(config_fn_t fn, void *); extern int git_config_with_options(config_fn_t fn, void *, struct git_config_source *config_source, int respect_includes); diff --git a/config.c b/config.c index e4d745e..4cefa25 100644 --- a/config.c +++ b/config.c @@ -1230,9 +1230,21 @@ int git_config_with_options(config_fn_t fn, void *data, return ret; } -int git_config(config_fn_t fn, void *data) +void git_config(config_fn_t fn, void *data) { - return git_config_with_options(fn, data, NULL, 1); + if (git_config_with_options(fn, data, NULL, 1) < 0) + /* +* git_config_with_options() normally returns only +* positive values, as most errors are fatal, and +* non-fatal potential errors are guarded by "if" +* statements that are entered only when no error is +* possible. +* +* If we ever encounter a non-fatal error, it means +* something went really wrong and we should stop +* immediately. +*/ + die(_("unknown error occured while reading the configuration files")); } static struct config_set_element *configset_find_element(struct config_set *cs, const char *key) -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 3/8] add line number and file name info to `config_set`
Store file name and line number for each key-value pair in the cache during parsing of the configuration files. Signed-off-by: Tanay Abhra --- cache.h | 5 + config.c | 16 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index 7292aef..0b1bdfd 100644 --- a/cache.h +++ b/cache.h @@ -1383,6 +1383,11 @@ extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest); extern int git_config_get_maybe_bool(const char *key, int *dest); extern int git_config_get_pathname(const char *key, const char **dest); +struct key_value_info { + const char *filename; + int linenr; +}; + extern int committer_ident_sufficiently_given(void); extern int author_ident_sufficiently_given(void); diff --git a/config.c b/config.c index bb4629e..e4d745e 100644 --- a/config.c +++ b/config.c @@ -1260,6 +1260,9 @@ static struct config_set_element *configset_find_element(struct config_set *cs, static int configset_add_value(struct config_set *cs, const char *key, const char *value) { struct config_set_element *e; + struct string_list_item *si; + struct key_value_info *kv_info = xmalloc(sizeof(*kv_info)); + e = configset_find_element(cs, key); /* * Since the keys are being fed by git_config*() callback mechanism, they @@ -1272,7 +1275,16 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha string_list_init(&e->value_list, 1); hashmap_add(&cs->config_hash, e); } - string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL); + si = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL); + if (cf) { + kv_info->filename = strintern(cf->name); + kv_info->linenr = cf->linenr; + } else { + /* for values read from `git_config_from_parameters()` */ + kv_info->filename = NULL; + kv_info->linenr = -1; + } + si->util = kv_info; return 0; } @@ -1299,7 +1311,7 @@ void git_configset_clear(struct config_set *cs) hashmap_iter_init(&cs->config_hash, &iter); while ((entry = hashmap_iter_next(&iter))) { free(entry->key); - string_list_clear(&entry->value_list, 0); + string_list_clear(&entry->value_list, 1); } hashmap_free(&cs->config_hash, 1); cs->hash_initialized = 0; -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 1/8] config.c: mark error and warnings strings for translation
From: Matthieu Moy Signed-off-by: Matthieu Moy --- config.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/config.c b/config.c index a191328..34940fd 100644 --- a/config.c +++ b/config.c @@ -457,9 +457,9 @@ static int git_parse_source(config_fn_t fn, void *data) break; } if (cf->die_on_error) - die("bad config file line %d in %s", cf->linenr, cf->name); + die(_("bad config file line %d in %s"), cf->linenr, cf->name); else - return error("bad config file line %d in %s", cf->linenr, cf->name); + return error(_("bad config file line %d in %s"), cf->linenr, cf->name); } static int parse_unit_factor(const char *end, uintmax_t *val) @@ -575,9 +575,9 @@ static void die_bad_number(const char *name, const char *value) value = ""; if (cf && cf->name) - die("bad numeric config value '%s' for '%s' in %s: %s", + die(_("bad numeric config value '%s' for '%s' in %s: %s"), value, name, cf->name, reason); - die("bad numeric config value '%s' for '%s': %s", value, name, reason); + die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason); } int git_config_int(const char *name, const char *value) @@ -662,7 +662,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value) return config_error_nonbool(var); *dest = expand_user_path(value); if (!*dest) - die("Failed to expand user dir in: '%s'", value); + die(_("failed to expand user dir in: '%s'"), value); return 0; } @@ -740,7 +740,7 @@ static int git_default_core_config(const char *var, const char *value) if (level == -1) level = Z_DEFAULT_COMPRESSION; else if (level < 0 || level > Z_BEST_COMPRESSION) - die("bad zlib compression level %d", level); + die(_("bad zlib compression level %d"), level); zlib_compression_level = level; zlib_compression_seen = 1; return 0; @@ -751,7 +751,7 @@ static int git_default_core_config(const char *var, const char *value) if (level == -1) level = Z_DEFAULT_COMPRESSION; else if (level < 0 || level > Z_BEST_COMPRESSION) - die("bad zlib compression level %d", level); + die(_("bad zlib compression level %d"), level); core_compression_level = level; core_compression_seen = 1; if (!zlib_compression_seen) @@ -873,7 +873,7 @@ static int git_default_core_config(const char *var, const char *value) else if (!strcmp(value, "link")) object_creation_mode = OBJECT_CREATION_USES_HARDLINKS; else - die("Invalid mode for object creation: %s", value); + die(_("invalid mode for object creation: %s"), value); return 0; } @@ -1173,7 +1173,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) switch (git_config_from_parameters(fn, data)) { case -1: /* error */ - die("unable to parse command-line config"); + die(_("unable to parse command-line config")); break; case 0: /* found nothing */ break; @@ -1514,7 +1514,7 @@ static int store_aux(const char *key, const char *value, void *cb) case KEY_SEEN: if (matches(key, value)) { if (store.seen == 1 && store.multi_replace == 0) { - warning("%s has multiple values", key); + warning(_("%s has multiple values"), key); } ALLOC_GROW(store.offset, store.seen + 1, -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 5/8] config: add `git_die_config()` to the config-set API
Add `git_die_config` that dies printing the line number and the file name of the highest priority value for the configuration variable `key`. A custom error message is also printed before dying, specified by the caller, which can be skipped if `err` argument is set to NULL. It has usage in non-callback based config value retrieval where we can raise an error and die if there is a semantic error. For example, if (!git_config_get_value(key, &value)){ if (!strcmp(value, "foo")) git_config_die(key, "value: `%s` is illegal", value); else /* do work */ } Signed-off-by: Tanay Abhra --- Documentation/technical/api-config.txt | 13 cache.h| 3 +++ config.c | 39 -- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt index 21f280c..0d8b99b 100644 --- a/Documentation/technical/api-config.txt +++ b/Documentation/technical/api-config.txt @@ -155,6 +155,19 @@ as well as retrieval for the queried variable, including: Similar to `git_config_get_string`, but expands `~` or `~user` into the user's home directory when found at the beginning of the path. +`git_die_config(const char *key, const char *err, ...)`:: + + First prints the error message specified by the caller in `err` and then + dies printing the line number and the file name of the highest priority + value for the configuration variable `key`. + +`void git_die_config_linenr(const char *key, const char *filename, int linenr)`:: + + Helper function which formats the die error message according to the + parameters entered. Used by `git_die_config()`. It can be used by callers + handling `git_config_get_value_multi()` to print the correct error message + for the desired value. + See test-config.c for usage examples. Value Parsing Helpers diff --git a/cache.h b/cache.h index f11ce41..89a0d51 100644 --- a/cache.h +++ b/cache.h @@ -1388,6 +1388,9 @@ struct key_value_info { int linenr; }; +extern NORETURN void git_die_config(const char *key, const char *err, ...) __attribute__((format(printf, 2, 3))); +extern NORETURN void git_die_config_linenr(const char *key, const char *filename, int linenr); + extern int committer_ident_sufficiently_given(void); extern int author_ident_sufficiently_given(void); diff --git a/config.c b/config.c index 4cefa25..5ae9ab0 100644 --- a/config.c +++ b/config.c @@ -1469,8 +1469,12 @@ const struct string_list *git_config_get_value_multi(const char *key) int git_config_get_string_const(const char *key, const char **dest) { + int ret; git_config_check_init(); - return git_configset_get_string_const(&the_config_set, key, dest); + ret = git_configset_get_string_const(&the_config_set, key, dest); + if (ret < 0) + git_die_config(key, NULL); + return ret; } int git_config_get_string(const char *key, char **dest) @@ -1511,8 +1515,39 @@ int git_config_get_maybe_bool(const char *key, int *dest) int git_config_get_pathname(const char *key, const char **dest) { + int ret; git_config_check_init(); - return git_configset_get_pathname(&the_config_set, key, dest); + ret = git_configset_get_pathname(&the_config_set, key, dest); + if (ret < 0) + git_die_config(key, NULL); + return ret; +} + +NORETURN +void git_die_config_linenr(const char *key, const char *filename, int linenr) +{ + if (!filename) + die(_("unable to parse '%s' from command-line config"), key); + else + die(_("bad config variable '%s' in file '%s' at line %d"), + key, filename, linenr); +} + +NORETURN __attribute__((format(printf, 2, 3))) +void git_die_config(const char *key, const char *err, ...) +{ + const struct string_list *values; + struct key_value_info *kv_info; + + if (err) { + va_list params; + va_start(params, err); + vreportf("error: ", err, params); + va_end(params); + } + values = git_config_get_value_multi(key); + kv_info = values->items[values->nr - 1].util; + git_die_config_linenr(key, kv_info->filename, kv_info->linenr); } /* -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v9 0/8] Rewrite `git_config()` using config-set API
[Patch v9]: Changed the grep statements in patch 7/8 and 8/8. [Patch v8]: git_die_config now allows custom error messages. new tests are now not too reliant on specific strings. [Patch v7]: style nit corrected. (1/8) is Matthieu's translation patch. git_die_config_linenr() helper function added. Diff between v6 and v7 appended for review. [Patch v6]: Added _() to error messages. Diff between v6 and v4 at the bottom. [PATCH v5]: New patch added (3/7). git_config() now returns void. [PATCH v4]: One style nit corrected, also added key to error messages. [PATCH V3]:All the suggestions in [3] applied. Built on top of [1]. [PATCH V2]: All the suggestions in [2] incorporated. git_config() now follows correct parsing order. Reordered the patches. Removed xfuncname patch as it was unnecssary. This series builds on the top of topic[1] in the mailing list with name "git config cache & special querying API utilizing the cache" or (ta/config-set in pu). This series aims to do these three things, * Use the config-set API to rewrite git_config(). * Solve any legacy bugs in the previous system while at it. * To be feature complete compared to the previous git_config() implementation, which I think it is now. (added the line number and file name info just for completeness) Also, I haven't yet checked the exact improvements but still as a teaser, git status now only rereads the configuration files twice instead of four times. [1]: http://thread.gmane.org/gmane.comp.version-control.git/254286 [2]: http://thread.gmane.org/gmane.comp.version-control.git/254101 [3]: http://thread.gmane.org/gmane.comp.version-control.git/254211 Matthieu Moy (1): config.c: mark error and warnings strings for translation Tanay Abhra (7): config.c: fix accuracy of line number in errors add line number and file name info to `config_set` change `git_config()` return value to void config: add `git_die_config()` to the config-set API rewrite git_config() to use the config-set API add a test for semantic errors in config files add tests for `git_config_get_string_const()` Documentation/technical/api-config.txt | 13 +++ branch.c | 5 +- cache.h| 34 +++- config.c | 152 +++-- t/t1308-config-set.sh | 21 + t/t4055-diff-context.sh| 2 +- test-config.c | 10 +++ 7 files changed, 207 insertions(+), 30 deletions(-) -- 1.9.0.GIT -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: "Branch objects" (was: Re: cherry picking and merge)
Nico Williams wrote: > On Wed, Aug 06, 2014 at 08:31:18PM +0200, Jakub Narębski wrote: > > On Wed, Aug 6, 2014 at 6:26 PM, Nico Williams wrote: > > > > > > My proposal was to put this sort of ancillary history info in a > > > "branch object" (and that branches should be objects). > > > > Is it something like object-ified reflog, similar to how replacement > > objects (git-replace) can be thought to be object-ified grafts (I know > > they are more)? Do I understand it correctly? > > Yes, per-branch. At push time a commit would be pushed to the upstream > branch listing the commits pushed now (and who by). Locally every > rebase/cherry-pick/merge/commit onto the branch would appear in the > branch object's history, kinda just like the reflog. The main > difference is that the upstream branch's history could be viewed. > > > With rebases the problem is that it would be nice to have (at least > > for a short time) the history of series of patches (the metahistory, > > or history of a branch), but usually one doesn't need old pre-rebase > > version after cleaning up the history for publishing. > > Right. I have been fiddling around in this area. What I want to be able to do is develop fixes for open source code that I run, and get those fixes upstream. This means I need a rebasing workflow, to keep the patches up-to-date and to deal with code review feedback. But this is inconvenient for deploying the patched version to production (which is the point of developing the fixes) - I want a fast-forwarding branch for that. And it would be nice to be able to share the history of the patch series, so others can see what changed between revisions more easily. So I have a small tool which maintains a publication branch which tracks the head of a rebasing branch. It's reasonably satisfactory so far... https://git.csx.cam.ac.uk/x/ucs/git/git-repub.git ... though the structure of the publication branch is weird and not very easy to navigate. You can see it in action in my git.git repo: https://git.csx.cam.ac.uk/x/ucs/git/git.git/shortlog/refs/heads/ucam/fanf2/patch Tony. -- f.anthony.n.finchhttp://dotat.at/ Irish Sea: Variable 4. Slight. Showers. Good.
Re: [PATCH v8 8/8] add tests for `git_config_get_string_const()`
Junio C Hamano writes: > Matthieu Moy writes: > >> Tanay Abhra writes: >> >>> ... >>> + grep "line 7.*.git/config\|.git/config.*line 7" result >>> +' >> >> This is still dependant on the locale ("line" is translated). You need >> to use test_i18ngrep instead of grep here (see its definition and >> comment in t/test-lib.sh). >> >> I don't think you need these two alternatives OTOH. >> >> BTW, Junio, I don't understand your remark "This test is too tight (the >> full string)" in the previous iteration. Can you elaborate? > > The original test was trying to match the string fully, i.e. > >> +grep "fatal: bad config variable '\''alias.br'\'' at file line 2 in >> .git/config" result > > As I already was feeling funny about seeing the phrase "file line" > in the message and expecting it to be corrected, I thought I should > encourage a check that does not depend on minor phrasing changes, if > it can be done without bending backwards. OK. I personally prefer tight tests in this case, as the patch doing the rephrase "sees" what changed by running the tests, and documents the visible change by changing the expected in the tests scripts. But no strong opinion here, I'd be fine with e.g. test_i18ngrep "fatal: .* alias.br.*line 2 in .git/config" result -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
Fabian Ruch: 2. Notice ourselves that the end-result of the whole squash is an empty commit, and stop to let the user deal with it. This patch chooses the second alternative. Either way seems OK. The crucial consensus of the discussion was to silently throw away empty interim commits. Yes, the important part is that giving good advice is better than giving bad advice. Thank you for taking your time to fix this. I haven't reviewed the changes themselves, but I am happy with the underlying idea. -- \\// Peter - http://www.softwolves.pp.se/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html