Ben Peart writes:
> @@ -344,6 +346,7 @@ struct index_state {
> struct hashmap dir_hash;
> unsigned char sha1[20];
> struct untracked_cache *untracked;
> + uint64_t fsmonitor_last_update;
This field being zero has more significance than just "we haven't
got any update yet",
Ben Peart writes:
> + OPT_SET_INT(0, "force-write-index", &force_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 when it actually m
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 item is unnecessarily
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 the untracked files in that stas
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 */
++item)
Even though this
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 ;-)
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 SOME_CONDITION
Jonathan Nieder writes:
> ...
> strerror(ENOSYS) is "Function not implemented". Cute.
>
> Reviewed-by: Jonathan Nieder
Thanks, both. Will queue.
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 clobber_head)
"update" to me mea
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 creates a reflog for the branch
>
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 take true or false in "int", and t
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 single pointer NULL-n
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 '.' that are meant to match only a
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:
>
> Here's a v2 that
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 for_each_string_list_item(item,list)
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", &core_fsmonitor))
> +
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 example
<>
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
> magical, but I think t
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 global. [...]
#define for_e
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 for_each_string_list_item(item,list) \
>>> - for (item = (list)
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 a
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 to
> -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;
> gits...@pobox.com; johannes.schinde...@gmx.de; pclo...@gmail.c
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 n
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)" (withou
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 ca
> -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;
> gits...@pobox.com; johannes.schinde...@gmx.de; pclo...@gmail.com;
>
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.
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
> +extern void unleak_m
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; git@vger.kernel.org; gits...@pobox.com;
joha
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, t
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 of these are por
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 hav
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
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 con
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 s
> -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;
> johannes.schinde...@gmx.de; pclo...@
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 fai
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 tho
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 fir
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; christian.cou...@gmail.com; git@vger.kern
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 t/helper/test-dump-fsmonitor
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 a
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
index e1339e6d17..313962
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
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 whic
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 7d063e9e40..6b22c46214 100644
--- a
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 -
Documentation/git-update-index.txt | 45
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.
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 file changed, 2 insertio
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 (--force-write-inde
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 +++
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 no
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 --[no-]fsmo
> 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 f
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 I
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 i
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 un
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
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 tha
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 e
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 workin
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 References: fields of the "Wha
> -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;
> christian.cou...@gmail.com; git@vger.kernel.org;
> johannes.schinde...@g
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,
> > ULONG))GetProcAddress(
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 the
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 + (list)-
>
> 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.
Le
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) \
> for (int f_e_s
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 macro.
>>
>> By which you a
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
---
Documen
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: Ka
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()` a
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 wor
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
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 gi
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
err
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 "Unifying Git's format langua
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 flag
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(+)
diff --git a/branch.h b/branch.
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 prepa
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 weren't
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
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
85 matches
Mail list logo
