Re: [PATCH v7 03/12] update-index: add a new --force-write-index option

Ben Peart writes: > + OPT_SET_INT(0, "force-write-index", _write, > + N_("write out the index even if is not flagged as > changed"), 1), Hmph. The only time this makes difference is when the code forgets to mark active_cache_changed even

Re: [PATCH v2] for_each_string_list_item: avoid undefined behavior for empty list

Jonathan Nieder writes: > D. Eliminate for_each_string_list_item and let callers just do > > unsigned int i; > for (i = 0; i < list->nr; i++) { > struct string_list_item *item = list->items[i]; > ... > } > >Having to declare

Re: Behaviour of 'git stash show' when a stash has untracked files

Kaartic Sivaraam writes: > Some time ago, I stashed a few changes along with untracked files. I > almost forgot it until recently. Then I wanted to see what I change I > had in the stash. So I did a 'git stash show '. It worked fine but > didn't say anything about

[PATCH v2] for_each_string_list_item: avoid undefined behavior for empty list

From: Michael Haggerty If you pass a newly initialized or newly cleared `string_list` to `for_each_string_list_item()`, then the latter does for ( item = (list)->items; /* NULL */ item < (list)->items + (list)->nr; /* NULL + 0 */

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

Jonathan Nieder writes: > I'll send a patch with a commit message saying so to try to close out > this discussion. Thanks. One less thing we have to worry about ;-)

Re: [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives

Jeff King writes: > At any rate, I think Jonathan's point is that writing: > > UNLEAK(foo) > > will silently pass in a normal build, and only much later will somebody > run a leak-checking build and see the compile error. Yeah, I think I understand that concern. #if

Re: [PATCH] Win32: simplify loading of DLL functions

Jonathan Nieder writes: > ... > strerror(ENOSYS) is "Function not implemented". Cute. > > Reviewed-by: Jonathan Nieder Thanks, both. Will queue.

Re: [RFC PATCH 3/5] branch: cleanup branch name validation

Kaartic Sivaraam writes: > -int validate_new_branchname(const char *name, struct strbuf *ref, > - int force, int attr_only) > +int validate_branch_update(const char *name, struct strbuf *ref, > +int could_exist, int

Re: [RFC PATCH 2/5] branch: document the usage of certain parameters

Kaartic Sivaraam writes: > Documentation for a certain function was incomplete as it didn't say > what certain parameters were used for. > > So, document them for the sake of completeness and easy reference. Thanks. > @@ -15,6 +15,11 @@ > * > * - reflog

Re: [RFC PATCH 1/5] builtin/checkout: avoid usage of '!!'

Kaartic Sivaraam writes: > There was a usage for which there's no compelling reason.So, replace > such a usage as with something else that expresses the intent more > clearly. I actually think this is a good example of the exception-rule. The function wants to

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

Jonathan Nieder writes: > ... But a quick test with gcc 4.8.4 > -O2 finds that at least this compiler does not contain such an > optimization. The overhead Michael Haggerty mentioned is real. Still, I have a feeling that users of string_list wouldn't care the overhead of

Re: [PATCH] t4014: strengthen search patterns

Kaartic Sivaraam writes: > The regex patterns for some failing test cases were a bit loose > giving way for a few incorrect outputs being accepted as correct > outputs. If these were part of scripted Porcelain that needs to take any end-user input, then having

Re: [PATCH v2 0/2] fix read past end of array in alternates files

Jeff King writes: > On Mon, Sep 18, 2017 at 11:51:00AM -0400, Jeff King wrote: > >> This series fixes a regression in v2.11.1 where we might read past the >> end of an mmap'd buffer. It was introduced in cf3c635210, but I didn't >> base the patch on there, for a few reasons: > >

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

Hi, Kaartic Sivaraam wrote: > On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: >> Jonathan Nieder wrote: >>> Does the following alternate fix work? I think I prefer it because >>> it doesn't require introducing a new global. [...] >>> #define

Re: [PATCH v7 04/12] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

Ben Peart writes: > +/* do stat comparison even if CE_FSMONITOR_VALID is true */ > +#define CE_MATCH_IGNORE_FSMONITOR 0X20 Hmm, when should a programmer use this flag? > +int git_config_get_fsmonitor(void) > +{ > + if (git_config_get_pathname("core.fsmonitor",

Re: [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives

On Wed, Sep 20, 2017 at 10:45:05AM +0900, Junio C Hamano wrote: > >> > +#else > >> > +#define UNLEAK(var) > >> > >> I think this should be defined to be something (for example, "do {} > >> while (0)"), at least so that we have compiler errors when UNLEAK(var) > >> is used incorrectly (for

[no subject]

<>

Re: [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives

Jeff King writes: > On Tue, Sep 19, 2017 at 01:45:52PM -0700, Jonathan Tan wrote: > >> The following comments are assuming that we're going to standardize on >> UNLEAK(var); (with the semicolon). > > Yeah, I assumed we would. We don't have to, since this really is sort-of >

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

Hi, Junio C Hamano wrote: > Kaartic Sivaraam writes: >> On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: >>> Jonathan Nieder wrote: Does the following alternate fix work? I think I prefer it because it doesn't require introducing a new

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

Kaartic Sivaraam writes: > On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: >>> Does the following alternate fix work? I think I prefer it because >>> it doesn't require introducing a new global. [...] >>> #define

[PATCH v2] describe: teach --match to handle branches and remotes

When `git describe` uses `--match`, it matches only tags, basically ignoring the `--all` argument even when it is specified. Fix it by also matching branch name and $remote_name/$remote_branch_name, for remote-tracking references, with the specified patterns. Update documentation accordingly and

Re: [PATCH] describe: teach --match to handle branches and remotes

On Tue, Sep 19, 2017 at 08:52:24AM +0900, Junio C Hamano wrote: > I think you can use skip_prefix() to avoid counting the length of > the prefix yourself, i.e. Thanks, will use it. > The hardcoded +10 for "is_tag" case assumes that anything other than > "refs/tags/something" would ever be used

RE: [PATCH v7 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit

> -Original Message- > From: David Turner [mailto:david.tur...@twosigma.com] > Sent: Tuesday, September 19, 2017 5:27 PM > To: 'Ben Peart' ; Ben Peart > > Cc: ava...@gmail.com; christian.cou...@gmail.com; git@vger.kernel.org; >

[PATCH for jk/leak-checkers v2] git-compat-util: make UNLEAK less error-prone

Commit 0e5bba5 ("add UNLEAK annotation for reducing leak false positives", 2017-09-08) introduced an UNLEAK macro to be used as "UNLEAK(var);", but its existing definitions leave semicolons that act as empty statements, which may cause some tools to complain. Therefore, modify its definitions to

Re: [PATCH for jk/leak-checkers] git-compat-util: make UNLEAK less error-prone

On Tue, Sep 19, 2017 at 02:34:56PM -0700, Jonathan Tan wrote: > Commit 0e5bba5 ("add UNLEAK annotation for reducing leak false > positives", 2017-09-08) introduced an UNLEAK macro to be used as > "UNLEAK(var);", but its existing definitions make it possible to be > invoked as "UNLEAK(var)"

[PATCH for jk/leak-checkers] git-compat-util: make UNLEAK less error-prone

Commit 0e5bba5 ("add UNLEAK annotation for reducing leak false positives", 2017-09-08) introduced an UNLEAK macro to be used as "UNLEAK(var);", but its existing definitions make it possible to be invoked as "UNLEAK(var)" (without the trailing semicolon) too. Therefore, modify its definitions to

RE: [PATCH v7 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit

> -Original Message- > From: Ben Peart [mailto:peart...@gmail.com] > Sent: Tuesday, September 19, 2017 4:45 PM > To: David Turner ; 'Ben Peart' > > Cc: ava...@gmail.com; christian.cou...@gmail.com; git@vger.kernel.org; >

Re: [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives

On Tue, Sep 19, 2017 at 01:45:52PM -0700, Jonathan Tan wrote: > The following comments are assuming that we're going to standardize on > UNLEAK(var); (with the semicolon). Yeah, I assumed we would. We don't have to, since this really is sort-of magical, but I think the code looks better with it.

Re: [PATCH v2 10/10] add UNLEAK annotation for reducing leak false positives

Thanks - this does look like a good thing to have. Sorry for the late comments. The following comments are assuming that we're going to standardize on UNLEAK(var); (with the semicolon). On Fri, 8 Sep 2017 02:38:41 -0400 Jeff King wrote: > +#ifdef SUPPRESS_ANNOTATED_LEAKS >

Re: [PATCH v7 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit

On 9/19/2017 3:46 PM, David Turner wrote: -Original Message- From: Ben Peart [mailto:benpe...@microsoft.com] Sent: Tuesday, September 19, 2017 3:28 PM To: benpe...@microsoft.com Cc: David Turner ; ava...@gmail.com; christian.cou...@gmail.com;

Re: [PATCH v6 09/12] split-index: disable the fsmonitor extension when running the split index test

Hi, Ben Peart wrote: > The split index test t1700-split-index.sh has hard coded SHA values for > the index. Currently it supports index V4 and V3 but assumes there are > no index extensions loaded. > > When manually forcing the fsmonitor extension to be turned on when > running the test suite,

Re: [PATCH v1 1/1] test-lint: echo -e (or -E) is not portable

Torsten Bögershausen wrote: > Some implementations of `echo` support the '-e' option to enable > backslash interpretation of the following string. > As an addition, they support '-E' to turn it off. nit: please wrap the commit message to a consistent line width. > However, none

Re: [PATCH v6 10/12] fsmonitor: add test cases for fsmonitor extension

Ben Peart wrote: > Some stats on these same coding style errors in the current bash scripts: > > 298 instances of "[a-z]\(\).*\{" ie "function_name() {" (no space) > 140 instances of "if \[ .* \]" (not using the preferred "test") > 293 instances of "if .*; then" > > Wouldn't it be great not to

Re: [PATCH v6 10/12] fsmonitor: add test cases for fsmonitor extension

On 9/19/2017 3:32 PM, David Turner wrote: I think my comment here might have gotten lost, and I don't want it to because it's something I'm really worried about: You have to update the test completely to ensure it passes. If you run the test with the '-v' option you will see the cause of

Re: [PATCH] doc: camelCase the config variables to improve readability

Hi, Kaartic Sivaraam wrote: > The config variable used weren't readable as they were in the > crude form of how git stores/uses it's config variables. nit: Git's commit messages describe the current behavior of Git in the present tense. Something like: These manpages' references to

Re: [PATCH v2 00/21] Read `packed-refs` using mmap()

Hi Michael, On Tue, 19 Sep 2017, Michael Haggerty wrote: > This is v2 of a patch series that changes the reading and caching of the > `packed-refs` file to use `mmap()`. Thanks to Junio, Stefan, and > Johannes for their comments about v1 [1]. Thank you for the new iteration. > The main change

RE: [PATCH v7 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit

> -Original Message- > From: Ben Peart [mailto:benpe...@microsoft.com] > Sent: Tuesday, September 19, 2017 3:28 PM > To: benpe...@microsoft.com > Cc: David Turner ; ava...@gmail.com; > christian.cou...@gmail.com; git@vger.kernel.org; gits...@pobox.com; >

[PATCH v2 2/2] read_info_alternates: warn on non-trivial errors

When we fail to open $GIT_DIR/info/alternates, we silently assume there are no alternates. This is the right thing to do for ENOENT, but not for other errors. A hard error is probably overkill here. If we fail to read an alternates file then either we'll complete our operation anyway, or we'll

[PATCH v2 1/2] read_info_alternates: read contents into strbuf

This patch fixes a regression in v2.11.1 where we might read past the end of an mmap'd buffer. It was introduced in cf3c635210. The link_alt_odb_entries() function has always taken a ptr/len pair as input. Until cf3c635210 (alternates: accept double-quoted paths, 2016-12-12), we made a copy of

[PATCH v2 0/2] fix read past end of array in alternates files

On Mon, Sep 18, 2017 at 11:51:00AM -0400, Jeff King wrote: > This series fixes a regression in v2.11.1 where we might read past the > end of an mmap'd buffer. It was introduced in cf3c635210, but I didn't > base the patch on there, for a few reasons: Here's a v2 that fixes a minor leak in the

RE: [PATCH v6 10/12] fsmonitor: add test cases for fsmonitor extension

I think my comment here might have gotten lost, and I don't want it to because it's something I'm really worried about: > -Original Message- > From: David Turner > Sent: Friday, September 15, 2017 6:00 PM > To: 'Ben Peart' > Cc: ava...@gmail.com;

[PATCH v7 08/12] fsmonitor: add a test tool to dump the index extension

Add a test utility (test-dump-fsmonitor) that will dump the fsmonitor index extension. Signed-off-by: Ben Peart --- Makefile | 1 + t/helper/test-dump-fsmonitor.c | 21 + 2 files changed, 22 insertions(+) create mode 100644

[PATCH v7 04/12] fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.

When the index is read from disk, the fsmonitor index extension is used to flag the last known potentially dirty index entries. The registered core.fsmonitor command is called with the time the index was last updated and returns the list of files changed since that time. This list is used to flag

[PATCH v7 06/12] ls-files: Add support in ls-files to display the fsmonitor valid bit

Add a new command line option (-f) to ls-files to have it use lowercase letters for 'fsmonitor valid' files Signed-off-by: Ben Peart --- builtin/ls-files.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c

[PATCH v7 10/12] fsmonitor: add test cases for fsmonitor extension

Test the ability to add/remove the fsmonitor index extension via update-index. Test that dirty files returned from the integration script are properly represented in the index extension and verify that ls-files correctly reports their state. Test that ensure status results are correct when using

[PATCH v7 09/12] split-index: disable the fsmonitor extension when running the split index test

The split index test t1700-split-index.sh has hard coded SHA values for the index. Currently it supports index V4 and V3 but assumes there are no index extensions loaded. When manually forcing the fsmonitor extension to be turned on when running the test suite, the SHA values no longer match

[PATCH v7 01/12] bswap: add 64 bit endianness helper get_be64

Add a new get_be64 macro to enable 64 bit endian conversions on memory that may or may not be aligned. Signed-off-by: Ben Peart --- compat/bswap.h | 22 ++ 1 file changed, 22 insertions(+) diff --git a/compat/bswap.h b/compat/bswap.h index

[PATCH v7 05/12] fsmonitor: add documentation for the fsmonitor extension.

This includes the core.fsmonitor setting, the query-fsmonitor hook, and the fsmonitor index extension. Signed-off-by: Ben Peart --- Documentation/config.txt | 7 + Documentation/git-ls-files.txt | 7 -

[PATCH v7 11/12] fsmonitor: add a sample integration script for Watchman

This script integrates the new fsmonitor capabilities of git with the cross platform Watchman file watching service. To use the script: Download and install Watchman from https://facebook.github.io/watchman/. Rename the sample integration hook from fsmonitor-watchman.sample to fsmonitor-watchman.

[PATCH v7 02/12] preload-index: add override to enable testing preload-index

Preload index doesn't run unless it has a minimum number of 1000 files. To enable running tests with fewer files, add an environment variable (GIT_FORCE_PRELOAD_TEST) which will override that minimum and set it to 2. Signed-off-by: Ben Peart --- preload-index.c | 2 ++ 1

[PATCH v7 03/12] update-index: add a new --force-write-index option

At times, it makes sense to avoid the cost of writing out the index when the only changes can easily be recomputed on demand. This causes problems when trying to write test cases to verify that state as they can't guarantee the state has been persisted to disk. Add a new option

[PATCH v7 07/12] update-index: add fsmonitor support to update-index

Add support in update-index to manually add/remove the fsmonitor extension via --[no-]fsmonitor flags. Add support in update-index to manually set/clear the fsmonitor valid bit via --[no-]fsmonitor-valid flags. Signed-off-by: Ben Peart --- builtin/update-index.c | 33

[PATCH v7 12/12] fsmonitor: add a performance test

Add a test utility (test-drop-caches) that flushes all changes to disk then drops file system cache on Windows, Linux, and OSX. Add a perf test (p7519-fsmonitor.sh) for fsmonitor. By default, the performance test will utilize the Watchman file system monitor if it is installed. If Watchman is

[PATCH v7 00/12] Fast git status via a file system watcher

Subject: Fast git status via a file system watcher Thanks to everyone who provided feedback. There are lots of minor style changes, documentation updates and a fixed leak. The only functional change is the addition of support to set/clear the fsmonitor valid bit via 'git update-index

Re: [PATCH] rev-parse: rev-parse: add --is-shallow-repository

> Hm, can you say more about the context? From a certain point of view, > it might make sense for that command to succeed instead: if the repo > is already unshallow, then why should't "fetch --unshallow" complain > instead of declaring victory? A fellow in #git on Freenode was writing a script

Re: [PATCH] Win32: simplify loading of DLL functions

Hi, Johannes Schindelin wrote: > Dynamic loading of DLL functions is duplicated in several places in Git > for Windows' source code. > > This patch adds a pair of macros to simplify the process: the > DECLARE_PROC_ADDR(, , , > ..) macro to be used at the beginning of a > code block, and the

Re: [PATCH v2] Improve performance of git status --ignored

On 09/18, Jameson Miller wrote: > Improve the performance of the directory listing logic when it wants to list > non-empty ignored directories. In order to show non-empty ignored directories, > the existing logic will recursively iterate through all contents of an ignored > directory. This change

Re: [PATCH v6 09/40] Add initial external odb support

I wonder if it's better to get a change like this (PATCH v6 09/40 and any of the previous patches that this depends on) in and then build on it rather than to review the whole patch set at a time. This would reduce ripple effects (of needing to change later patches in a patch set multiple times

Re: What's cooking in git.git (Aug 2017, #05; Tue, 22)

Le 19/09/2017 à 17:43, Johannes Schindelin a écrit : > > C'mon, don't *try* to misunderstand me. > > Of course there need to be updates as to the state of patch series. > > It's just that mails only go *so* far when you need to connect and > aggregate information. You need the connection between

Re: RFC v3: Another proposed hash function transition plan

Hi Johannes, Thanks for your feedback. On 19/09/17 00:16, Johannes Schindelin wrote: >>> SHA-256 got much more cryptanalysis than SHA3-256 […]. >> >> I do not think this is true. > > Please read what I said again: SHA-256 got much more cryptanalysis > than SHA3-256. Indeed. What I meant is

Re: What's cooking in git.git (Aug 2017, #05; Tue, 22)

Johannes Schindelin wrote: > What you need is a tool to aggregate this information, to help working > with it, to manage the project, and to be updated automatically. And to > publish this information continuously, without costing you extra effort. > > I understand that you started before GitHub

Re: What's cooking in git.git (Aug 2017, #05; Tue, 22)

Hi, Johannes Schindelin wrote: > To relate that, you are using a plain text format that is not well defined > and not structured, and certainly not machine-readable, for information > that is crucial for project management. > > What you need is a tool to aggregate this information, to help

Re: What's cooking in git.git (Aug 2017, #05; Tue, 22)

Hi Junio, On Tue, 19 Sep 2017, Junio C Hamano wrote: > Johannes Schindelin writes: > > >> Do you have a concrete suggestion to make these individual entries > >> more helpful for people who may want go back to the original thread > >> in doing so? In-reply-to: or

RE: [PATCH v6 08/12] fsmonitor: add a test tool to dump the index extension

> -Original Message- > From: Torsten Bögershausen [mailto:tbo...@web.de] > Sent: Tuesday, September 19, 2017 10:16 AM > To: Ben Peart > Cc: Ben Peart ; Junio C Hamano > ; david.tur...@twosigma.com; ava...@gmail.com; >

RE: [PATCH v6 12/12] fsmonitor: add a performance test

Hi Ben, On Mon, 18 Sep 2017, Ben Peart wrote: > > From: Johannes Schindelin [mailto:johannes.schinde...@gmx.de] > > On Fri, 15 Sep 2017, Ben Peart wrote: > > > > > + DWORD(WINAPI *NtSetSystemInformation)(INT, PVOID, ULONG) = > > > + (DWORD(WINAPI *)(INT, PVOID, > >

[PATCH] Win32: simplify loading of DLL functions

Dynamic loading of DLL functions is duplicated in several places in Git for Windows' source code. This patch adds a pair of macros to simplify the process: the DECLARE_PROC_ADDR(, , , ..) macro to be used at the beginning of a code block, and the INIT_PROC_ADDR() macro to call before using

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

On Saturday 16 September 2017 09:36 AM, Michael Haggerty wrote: Does the following alternate fix work? I think I prefer it because it doesn't require introducing a new global. [...] #define for_each_string_list_item(item,list) \ - for (item = (list)->items; item < (list)->items +

Re: [PATCH v6 08/12] fsmonitor: add a test tool to dump the index extension

> > Should I just make the variable type itself uintmax_t and then just skip > the cast altogether? I went with uint64_t because that is what > getnanotime returned. > That is a bit of taste question (or answer) Typically you declare the variables in the type you need, and this is uint64_t.

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

On Tue, Sep 19, 2017 at 3:38 PM, SZEDER Gábor wrote: > A bit "futuristic" option along these lines could be something like > this, using a scoped loop variable in the outer loop to ensure that > it's executed at most once: > > #define for_each_string_list_item(item,list) \

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

On Tue, Sep 19, 2017 at 8:51 AM, Michael Haggerty wrote: > On 09/19/2017 02:08 AM, Stefan Beller wrote: >>> I am hoping that this last one is not allowed and we can use the >>> "same condition is checked every time we loop" version that hides >>> the uglyness inside the

[PATCH] doc: camelCase the config variables to improve readability

The config variable used weren't readable as they were in the crude form of how git stores/uses it's config variables. Improve it's readability by replacing them with camelCased versions of config variables as it doesn't have any impact on it's usage. Signed-off-by: Kaartic Sivaraam

[PATCH] t4014: strengthen search patterns

The regex patterns for some failing test cases were a bit loose giving way for a few incorrect outputs being accepted as correct outputs. To avoid such incorrect outputs from being flagged as correct ones use fixed string matches when possible and strengthen regex when it's not. Signed-off-by:

Re: [PATCH v2 13/21] packed_ref_cache: keep the `packed-refs` file mmapped if possible

On 09/19/2017 08:22 AM, Michael Haggerty wrote: > Keep a copy of the `packed-refs` file contents in memory for as long > as a `packed_ref_cache` object is in use: > > * If the system allows it, keep the `packed-refs` file mmapped. > > * If not (either because the system doesn't support `mmap()`

I Want To Know Your Opinion In Doing This Project.

My Dear, >From Mrs. Loveth Konnia. I belief that you can help in setting up a charity foundation for the benefit of mankind, I wish to establish a charity foundation to help the poor in your country under your care, Can you help to build this project in your country? Together We can make the

Re: [RFC SAMPLE] builtin/branch: give more useful error messages when renaming

Sorry, the email client seems to have crapped up the formatting. In case it looks difficult to follow, let me know so that I could send a better version. --- Kaartic

Re: [PATCH] doc: update information about windows build

On Monday 18 September 2017 12:32 AM, Phillip Wood wrote: May be the Windows build exit with failure on other repos rather than saying it passes? I'm not quite sure what you're asking. If the tests aren't run it needs to look like a pass or everyone's branches would be marked as failing on

[RFC SAMPLE] builtin/branch: give more useful error messages when renaming

The patch series results in a change in output as specified below. Only few cases have been shown here to keep it short. The output for other cases are similar. $ git branch * master foo bar Before patch, $ # Trying to rename non-existent branch $ git branch -m hypothet no_such_branch

Re: My first contribution

Hi Olga, On Tue, Sep 19, 2017 at 8:44 AM, Оля Тележная wrote: > Hello Jeff, > I want to try myself into new role of a Git contributor. Welcome to Git! > All of the projects sound super interesting for me and I am ready to take > part in all of them, but the project

[RFC PATCH 4/5] branch: introduce dont_fail parameter for update validation

This parameter allows the branch update validation function to optionally return a flag specifying the reason for failure, when requested. This allows the caller to know why it was about to die. This allows more useful error messages to be given to the user when trying to rename a branch. The

[RFC PATCH 2/5] branch: document the usage of certain parameters

Documentation for a certain function was incomplete as it didn't say what certain parameters were used for. So, document them for the sake of completeness and easy reference. Signed-off-by: Kaartic Sivaraam --- branch.h | 5 + 1 file changed, 5 insertions(+)

[RFC PATCH 0/5] branch: improve error messages of branch renaming

In builtin/branch, the error messages weren't handled directly by the branch renaming function and was left to the other function. Though this avoids redundancy this gave unclear error messages in some cases. So, make builtin/branch give more useful error messages. The first two patches are

[RFC PATCH 3/5] branch: cleanup branch name validation

The function that validates a new branch name was clumsy because, 1. It did more than just validating the branch name 2. Despite it's name, it is more often than not used to validate existing branch names through the 'force' and 'attr_only' parameters (whose names by the way

[RFC PATCH 5/5] builtin/branch: give more useful error messages when renaming

When trying to rename an inexistent branch to an existing branch the rename failed specifying the new branch name exists rather than specifying that the branch trying to be renamed doesn't exist. $ git branch -m tset master fatal: A branch named 'master' already exists. It's conventional

[RFC PATCH 1/5] builtin/checkout: avoid usage of '!!'

Documentation/CodingGuidelines says, "Some clever tricks, like using the !! operator with arithmetic constructs, can be extremely confusing to others. Avoid them, unless there is a compelling reason to use them." There was a usage for which there's no compelling reason.So, replace

Re: [PATCH] for_each_string_list_item(): behave correctly for empty list

On 09/19/2017 02:08 AM, Stefan Beller wrote: >> I am hoping that this last one is not allowed and we can use the >> "same condition is checked every time we loop" version that hides >> the uglyness inside the macro. > > By which you are referring to Jonathans solution posted. > Maybe we can

Re: [PATCH v2 1/4] mktag: add option which allows the tagger field to be omitted

On Tue, 2017-09-19 at 12:01 +0900, Junio C Hamano wrote: > > Hmph.  I cannot shake this nagging feeling that this is probably a > solution that is overly narrow to a single problem that won't scale > into the future. > > [...snip good point...] > > If we drop the "verification" step from the

[PATCH v2 21/21] packed-backend.c: rename a bunch of things and update comments

We've made huge changes to this file, and some of the old names and comments are no longer very fitting. So rename a bunch of things: * `struct packed_ref_cache` → `struct snapshot` * `acquire_packed_ref_cache()` → `acquire_snapshot()` * `release_packed_ref_buffer()` → `clear_snapshot_buffer()` *

[PATCH v2 03/21] packed_ref_cache: add a backlink to the associated `packed_ref_store`

It will prove convenient in upcoming patches. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index e411501871..a3d9210cb0

[PATCH v2 19/21] ref_cache: remove support for storing peeled values

Now that the `packed-refs` backend doesn't use `ref_cache`, there is nobody left who might want to store peeled values of references in `ref_cache`. So remove that feature. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 9 - refs/ref-cache.c | 42

[PATCH v2 06/21] read_packed_refs(): only check for a header at the top of the file

This tightens up the parsing a bit; previously, stray header-looking lines would have been processed. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 35 --- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git

[PATCH v2 17/21] ref_store: implement `refs_peel_ref()` generically

We're about to stop storing packed refs in a `ref_cache`. That means that the only way we have left to optimize `peel_ref()` is by checking whether the reference being peeled is the one currently being iterated over (in `current_ref_iter`), and if so, using `ref_iterator_peel()`. But this can be

[PATCH v2 01/21] ref_iterator: keep track of whether the iterator output is ordered

References are iterated over in order by refname, but reflogs are not. Some consumers of reference iteration care about the difference. Teach each `ref_iterator` to keep track of whether its output is ordered. `overlay_ref_iterator` is one of the picky consumers. Add a sanity check in

[PATCH v2 07/21] read_packed_refs(): make parsing of the header line more robust

The old code parsed the traits in the `packed-refs` header by looking for the string " trait " (i.e., the name of the trait with a space on either side) in the header line. This is fragile, because if any other implementation of Git forgets to write the trailing space, the last trait would

[PATCH v2 12/21] packed-backend.c: reorder some definitions

No code has been changed. This will make subsequent patches more self-contained. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 48 1 file changed, 24 insertions(+), 24 deletions(-) diff --git

[PATCH v2 20/21] mmapped_ref_iterator: inline into `packed_ref_iterator`

Since `packed_ref_iterator` is now delegating to `mmapped_ref_iterator` rather than `cache_ref_iterator` to do the heavy lifting, there is no need to keep the two iterators separate. So "inline" `mmapped_ref_iterator` into `packed_ref_iterator`. This removes a bunch of boilerplate. Signed-off-by:

[PATCH v2 09/21] packed_ref_cache: remember the file-wide peeling state

Rather than store the peeling state (i.e., the one defined by traits in the `packed-refs` file header line) in a local variable in `read_packed_refs()`, store it permanently in `packed_ref_cache`. This will be needed when we stop reading all packed refs at once. Signed-off-by: Michael Haggerty

[PATCH v2 16/21] packed_read_raw_ref(): read the reference from the mmapped buffer

Instead of reading the reference from the `ref_cache`, read it directly from the mmapped buffer. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/refs/packed-backend.c

[PATCH v2 11/21] mmapped_ref_iterator_advance(): no peeled value for broken refs

If a reference is broken, suppress its peeled value. Signed-off-by: Michael Haggerty --- refs/packed-backend.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 312116a99d..724c88631d 100644

[PATCH v2 00/21] Read `packed-refs` using mmap()

This is v2 of a patch series that changes the reading and caching of the `packed-refs` file to use `mmap()`. Thanks to Junio, Stefan, and Johannes for their comments about v1 [1]. The main change since v1 is to accommodate Windows, which doesn't let you replace a file using `rename()` if the file

[PATCH v2 02/21] prefix_ref_iterator: break when we leave the prefix

From: Jeff King If the underlying iterator is ordered, then `prefix_ref_iterator` can stop as soon as it sees a refname that comes after the prefix. This will rarely make a big difference now, because `ref_cache_iterator` only iterates over the directory containing the prefix (and

  1   2   >