* Re: Bug report: $program_name in error message
From: Stefan Beller @ 2016-12-18 21:34 UTC (permalink / raw)
To: Josh Bleecher Snyder; +Cc: git@vger.kernel.org
In-Reply-To: <CAFAcib8yauNRB6UbO8qS2_Ff4fDt-7mMsmgop8d1V0j=RoTBSA@mail.gmail.com>
On Sun, Dec 18, 2016 at 12:36 PM, Josh Bleecher Snyder
<josharian@gmail.com> wrote:
> To reproduce, run 'git submodule' from within a bare repo. Result:
>
> $ git submodule
> fatal: $program_name cannot be used without a working tree.
>
> Looks like the intent was for $program_name to be interpolated.
Which version of git do you use?
>
>
> As an aside, I sent a message a few days ago about a segfault when
> working with a filesystem with direct_io on, but it appears not to
> have made it to the archives on marc.info. Am I perhaps still
> greylisted?
Both emails show up in my mailbox (subscribed to the mailing list),
so I think that just nobody answered your first email as the answer
may be non trivial.
>
> Thanks,
> Josh
^ permalink raw reply
* Bug report: $program_name in error message
From: Josh Bleecher Snyder @ 2016-12-18 20:36 UTC (permalink / raw)
To: git
To reproduce, run 'git submodule' from within a bare repo. Result:
$ git submodule
fatal: $program_name cannot be used without a working tree.
Looks like the intent was for $program_name to be interpolated.
As an aside, I sent a message a few days ago about a segfault when
working with a filesystem with direct_io on, but it appears not to
have made it to the archives on marc.info. Am I perhaps still
greylisted?
Thanks,
Josh
^ permalink raw reply
* [PATCH v1] git-p4: add diff/merge properties to .gitattributes for GitLFS files
From: larsxschneider @ 2016-12-18 19:01 UTC (permalink / raw)
To: git; +Cc: luke, gitster, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
The `git lfs track` command generates a .gitattributes file with diff
and merge properties [1]. Set the same .gitattributes format for files
tracked with GitLFS in git-p4.
[1] https://github.com/git-lfs/git-lfs/blob/v1.5.3/commands/command_track.go#L121
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Notes:
Base Commit: d1271bddd4 (v2.11.0)
Diff on Web: https://github.com/git/git/compare/d1271bddd4...larsxschneider:e045b3d5c8
Checkout: git fetch https://github.com/larsxschneider/git git-p4/fix-lfs-attributes-v1 && git checkout e045b3d5c8
git-p4.py | 4 ++--
t/t9824-git-p4-git-lfs.sh | 24 ++++++++++++------------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index fd5ca52462..87b6932c81 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1098,10 +1098,10 @@ class GitLFS(LargeFileSystem):
'# Git LFS (see https://git-lfs.github.com/)\n',
'#\n',
] +
- ['*.' + f.replace(' ', '[[:space:]]') + ' filter=lfs -text\n'
+ ['*.' + f.replace(' ', '[[:space:]]') + ' filter=lfs diff=lfs merge=lfs -text\n'
for f in sorted(gitConfigList('git-p4.largeFileExtensions'))
] +
- ['/' + f.replace(' ', '[[:space:]]') + ' filter=lfs -text\n'
+ ['/' + f.replace(' ', '[[:space:]]') + ' filter=lfs diff=lfs merge=lfs -text\n'
for f in sorted(self.largeFiles) if not self.hasLargeFileExtension(f)
]
)
diff --git a/t/t9824-git-p4-git-lfs.sh b/t/t9824-git-p4-git-lfs.sh
index 110a7e7924..1379db6357 100755
--- a/t/t9824-git-p4-git-lfs.sh
+++ b/t/t9824-git-p4-git-lfs.sh
@@ -81,9 +81,9 @@ test_expect_success 'Store files in LFS based on size (>24 bytes)' '
#
# Git LFS (see https://git-lfs.github.com/)
#
- /file2.dat filter=lfs -text
- /file4.bin filter=lfs -text
- /path[[:space:]]with[[:space:]]spaces/file3.bin filter=lfs -text
+ /file2.dat filter=lfs diff=lfs merge=lfs -text
+ /file4.bin filter=lfs diff=lfs merge=lfs -text
+ /path[[:space:]]with[[:space:]]spaces/file3.bin filter=lfs diff=lfs merge=lfs -text
EOF
test_path_is_file .gitattributes &&
test_cmp expect .gitattributes
@@ -109,7 +109,7 @@ test_expect_success 'Store files in LFS based on size (>25 bytes)' '
#
# Git LFS (see https://git-lfs.github.com/)
#
- /file4.bin filter=lfs -text
+ /file4.bin filter=lfs diff=lfs merge=lfs -text
EOF
test_path_is_file .gitattributes &&
test_cmp expect .gitattributes
@@ -135,7 +135,7 @@ test_expect_success 'Store files in LFS based on extension (dat)' '
#
# Git LFS (see https://git-lfs.github.com/)
#
- *.dat filter=lfs -text
+ *.dat filter=lfs diff=lfs merge=lfs -text
EOF
test_path_is_file .gitattributes &&
test_cmp expect .gitattributes
@@ -163,8 +163,8 @@ test_expect_success 'Store files in LFS based on size (>25 bytes) and extension
#
# Git LFS (see https://git-lfs.github.com/)
#
- *.dat filter=lfs -text
- /file4.bin filter=lfs -text
+ *.dat filter=lfs diff=lfs merge=lfs -text
+ /file4.bin filter=lfs diff=lfs merge=lfs -text
EOF
test_path_is_file .gitattributes &&
test_cmp expect .gitattributes
@@ -199,8 +199,8 @@ test_expect_success 'Remove file from repo and store files in LFS based on size
#
# Git LFS (see https://git-lfs.github.com/)
#
- /file2.dat filter=lfs -text
- /path[[:space:]]with[[:space:]]spaces/file3.bin filter=lfs -text
+ /file2.dat filter=lfs diff=lfs merge=lfs -text
+ /path[[:space:]]with[[:space:]]spaces/file3.bin filter=lfs diff=lfs merge=lfs -text
EOF
test_path_is_file .gitattributes &&
test_cmp expect .gitattributes
@@ -237,8 +237,8 @@ test_expect_success 'Add .gitattributes and store files in LFS based on size (>2
#
# Git LFS (see https://git-lfs.github.com/)
#
- /file2.dat filter=lfs -text
- /path[[:space:]]with[[:space:]]spaces/file3.bin filter=lfs -text
+ /file2.dat filter=lfs diff=lfs merge=lfs -text
+ /path[[:space:]]with[[:space:]]spaces/file3.bin filter=lfs diff=lfs merge=lfs -text
EOF
test_path_is_file .gitattributes &&
test_cmp expect .gitattributes
@@ -278,7 +278,7 @@ test_expect_success 'Add big files to repo and store files in LFS based on compr
#
# Git LFS (see https://git-lfs.github.com/)
#
- /file6.bin filter=lfs -text
+ /file6.bin filter=lfs diff=lfs merge=lfs -text
EOF
test_path_is_file .gitattributes &&
test_cmp expect .gitattributes
--
2.11.0
^ permalink raw reply related
* [PATCH v1] git-p4: fix git-p4.pathEncoding for removed files
From: larsxschneider @ 2016-12-18 17:51 UTC (permalink / raw)
To: git; +Cc: luke, gitster, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
In a9e38359e3 we taught git-p4 a way to re-encode path names from what
was used in Perforce to UTF-8. This path re-encoding worked properly for
"added" paths. "Removed" paths were not re-encoded and therefore
different from the "added" paths. Consequently, these files were not
removed in a git-p4 cloned Git repository because the path names did not
match.
Fix this by moving the re-encoding to a place that affects "added" and
"removed" paths. Add a test to demonstrate the issue.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Notes:
Base Commit: d1271bddd4 (v2.11.0)
Diff on Web: https://github.com/git/git/compare/d1271bddd4...larsxschneider:05a82caa69
Checkout: git fetch https://github.com/larsxschneider/git git-p4/fix-path-encoding-v1 && git checkout 05a82caa69
git-p4.py | 19 +++++++++----------
t/t9822-git-p4-path-encoding.sh | 16 ++++++++++++++++
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index fd5ca52462..8f311cb4e8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2366,6 +2366,15 @@ class P4Sync(Command, P4UserMap):
break
path = wildcard_decode(path)
+ try:
+ path.decode('ascii')
+ except:
+ encoding = 'utf8'
+ if gitConfig('git-p4.pathEncoding'):
+ encoding = gitConfig('git-p4.pathEncoding')
+ path = path.decode(encoding, 'replace').encode('utf8', 'replace')
+ if self.verbose:
+ print 'Path with non-ASCII characters detected. Used %s to encode: %s ' % (encoding, path)
return path
def splitFilesIntoBranches(self, commit):
@@ -2495,16 +2504,6 @@ class P4Sync(Command, P4UserMap):
text = regexp.sub(r'$\1$', text)
contents = [ text ]
- try:
- relPath.decode('ascii')
- except:
- encoding = 'utf8'
- if gitConfig('git-p4.pathEncoding'):
- encoding = gitConfig('git-p4.pathEncoding')
- relPath = relPath.decode(encoding, 'replace').encode('utf8', 'replace')
- if self.verbose:
- print 'Path with non-ASCII characters detected. Used %s to encode: %s ' % (encoding, relPath)
-
if self.largeFileSystem:
(git_mode, contents) = self.largeFileSystem.processContent(git_mode, relPath, contents)
diff --git a/t/t9822-git-p4-path-encoding.sh b/t/t9822-git-p4-path-encoding.sh
index 7b83e696a9..c78477c19b 100755
--- a/t/t9822-git-p4-path-encoding.sh
+++ b/t/t9822-git-p4-path-encoding.sh
@@ -51,6 +51,22 @@ test_expect_success 'Clone repo containing iso8859-1 encoded paths with git-p4.p
)
'
+test_expect_success 'Delete iso8859-1 encoded paths and clone' '
+ (
+ cd "$cli" &&
+ ISO8859="$(printf "$ISO8859_ESCAPED")" &&
+ p4 delete "$ISO8859" &&
+ p4 submit -d "remove file"
+ ) &&
+ git p4 clone --destination="$git" //depot@all &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git -c core.quotepath=false ls-files >actual &&
+ test_must_be_empty actual
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] winansi_isatty(): fix when Git is used from CMD
From: Johannes Sixt @ 2016-12-18 15:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Pranit Bauva
In-Reply-To: <ffc6a7a0-4ae4-b755-0b09-5bcd7114a2e6@kdbg.org>
Am 18.12.2016 um 16:26 schrieb Johannes Sixt:
> The new isatty() override implemented by cbb3f3c9b197 (mingw: intercept
> isatty() to handle /dev/null as Git expects it, 2016-12-11) does not
> take into account that _get_osfhandle() returns the handle visible by
> the C code, which is the pipe. But it actually wants to investigate the
> properties of the handle that is actually connected to the outside
> world. Fortunately, there is already winansi_get_osfhandle(), which
> returns exactly this handle. Use it.
But quite frankly, I find the implementation of winansi_isatty()
very unsatisfactory.
I understand that you wanted to be defensive and to override the
decision made by MSVCRT only when necessary.
However!
winansi.c is all about overriding MSVCRT's console handling. If we are
connected to a console, then by the time isatty() is called (from
outside the emulation layer), all handling of file descriptors 1 and 2
is already outside MSVCRT's control. In particular, we have determined
unambiguously whether a terminal is connected (see is_console()). I
suggest to have the implementation below (on top of the patch I'm
responding to).
What do you think?
diff --git a/compat/winansi.c b/compat/winansi.c
index ba360be69b..1748d17777 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -575,9 +575,8 @@ static void detect_msys_tty(int fd)
int winansi_isatty(int fd)
{
- int res = isatty(fd);
-
- if (res) {
+ switch (fd) {
+ case 0:
/*
* Make sure that /dev/null is not fooling Git into believing
* that we are connected to a terminal, as "_isatty() returns a
@@ -586,21 +585,19 @@ int winansi_isatty(int fd)
*
* https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx
*/
- HANDLE handle = winansi_get_osfhandle(fd);
- if (fd == STDIN_FILENO) {
+ {
+ HANDLE handle = (HANDLE)_get_osfhandle(fd);
DWORD dummy;
- if (!GetConsoleMode(handle, &dummy))
- res = 0;
- } else if (fd == STDOUT_FILENO || fd == STDERR_FILENO) {
- CONSOLE_SCREEN_BUFFER_INFO dummy;
-
- if (!GetConsoleScreenBufferInfo(handle, &dummy))
- res = 0;
+ return !!GetConsoleMode(handle, &dummy);
}
+ case 1:
+ return !!hconsole1;
+ case 2:
+ return !!hconsole2;
}
- return res;
+ return isatty(fd);
}
void winansi_init(void)
--
2.11.0.79.gf6b77ca
^ permalink raw reply related
* [PATCH] winansi_isatty(): fix when Git is used from CMD
From: Johannes Sixt @ 2016-12-18 15:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Pranit Bauva
In-Reply-To: <5977e71d-da58-7cb0-bc69-343bb3a1341d@kdbg.org>
The code in winansi.c emulates ANSI escape sequences when Git is
connected to the "real" windows console, CMD.exe. The details are
outline in eac14f8909d9 (Win32: Thread-safe windows console output,
2012-01-14). Essentially, it plugs a pipe between C code and the actual
console output handle.
This commit also added an override for isatty(), but it was made
unnecessary by fcd428f4a952 (Win32: fix broken pipe detection,
2012-03-01).
The new isatty() override implemented by cbb3f3c9b197 (mingw: intercept
isatty() to handle /dev/null as Git expects it, 2016-12-11) does not
take into account that _get_osfhandle() returns the handle visible by
the C code, which is the pipe. But it actually wants to investigate the
properties of the handle that is actually connected to the outside
world. Fortunately, there is already winansi_get_osfhandle(), which
returns exactly this handle. Use it.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
I was able to test the idea earlier than anticipated and it does work
for me.
compat/winansi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/winansi.c b/compat/winansi.c
index cb725fb02f..ba360be69b 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -586,7 +586,7 @@ int winansi_isatty(int fd)
*
* https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx
*/
- HANDLE handle = (HANDLE)_get_osfhandle(fd);
+ HANDLE handle = winansi_get_osfhandle(fd);
if (fd == STDIN_FILENO) {
DWORD dummy;
--
2.11.0.79.gf6b77ca
^ permalink raw reply related
* Re: Suggestion for the "Did you mean this?" feature
From: Alexei Lozovsky @ 2016-12-18 15:16 UTC (permalink / raw)
To: Kaartic Sivaraam; +Cc: git
In-Reply-To: <1482063500.10858.1.camel@gmail.com>
On 18 December 2016 at 14:18, Kaartic Sivaraam wrote:
> Hello all,
>
> I have found the "Did you mean this?" feature of git as a very good
> feature. I thought it would be even better if it took a step toward by
> asking for a prompt when there was only one alternative to the command
> that was entered.
>
> E.g.
>
>> unique@unique-pc:~$ git hepl
>> git: 'hepl' is not a git command. See 'git --help'.
>>
>> Did you mean this?
>> help
>> [yes/No] : y
>> usage: git [--version] [--help] [-C <path>] [-c name=value]
>> [--exec-path[=<path>]] [--html-path] [--man-path] [--info-
>> path]
>> ....
>
> This would make it even better for the user as it would avoid having to
> correct the mistake long commands that had only a single error
> (considering history feature is enabled).
>
> Is this is a good idea ?
It's definitely a good thing for human users. For example, I am annoyed
from time to time when I type in some long spell, mistype one minor thing,
and the whole command fails. Then I need to press <up>, correct the
obvious typo, and run the command again.
Though, there is one aspect which may be the reason why git does not have
this feature: it requires interactive input. For example, it won't work
if some script tries to run an invalid git command. And git cannot really
tell whether it is running interactively or in a batch mode. If it is
running in batch mode then the whole script may hang indefinitely waiting
for nonexistent input. This also may apply to using git with pipes.
Maybe a configuration option or some GIT_NO_PROMPT environment variable
may be used to force disable this, but it still will be a hassle for the
scripts.
^ permalink raw reply
* Re: Suggestion for the "Did you mean this?" feature
From: Kaartic Sivaraam @ 2016-12-18 13:26 UTC (permalink / raw)
To: Stephan Beyer, git
In-Reply-To: <5e1a3c4b-43b9-29f2-68fe-8149d9940123@gmx.net>
On Sun, 2016-12-18 at 14:16 +0100, Stephan Beyer wrote:
> I cannot tell if this is a good idea (or why it would be a bad idea)
> but
> why do you restrict your suggestion to the case when there is only
> one
> alternative?
>
> Why not also something like:
>
> ---
> $ git sta
> git: 'sta' is not a git command. See 'git --help'.
>
> Did you mean one of these?
> [1] status
> [2] stage
> [3] stash
> You can choose or quit [1,2,3,q]:
That would be fine too. Just thought it would be a good start to start
with a simple case. Also, I wasn't sure if there were any drawback's
that I was missing. I guess if it was implemented it wouldn't be
difficult to extend it further.
--
Regards,
Kaartic
^ permalink raw reply
* Re: Suggestion for the "Did you mean this?" feature
From: Stephan Beyer @ 2016-12-18 13:16 UTC (permalink / raw)
To: Kaartic Sivaraam, git
In-Reply-To: <1482063500.10858.1.camel@gmail.com>
Hi,
On 12/18/2016 01:18 PM, Kaartic Sivaraam wrote:
> I have found the "Did you mean this?" feature of git as a very good
> feature. I thought it would be even better if it took a step toward by
> asking for a prompt when there was only one alternative to the command
> that was entered.
>
> E.g.
>
>> unique@unique-pc:~$ git hepl
>> git: 'hepl' is not a git command. See 'git --help'.
>>
>> Did you mean this?
>> help
>> [yes/No] : y
>> usage: git [--version] [--help] [-C <path>] [-c name=value]
>> [--exec-path[=<path>]] [--html-path] [--man-path] [--info-
>> path]
>> ....
>
> This would make it even better for the user as it would avoid having to
> correct the mistake long commands that had only a single error
> (considering history feature is enabled).
>
> Is this is a good idea ?
I cannot tell if this is a good idea (or why it would be a bad idea) but
why do you restrict your suggestion to the case when there is only one
alternative?
Why not also something like:
---
$ git sta
git: 'sta' is not a git command. See 'git --help'.
Did you mean one of these?
[1] status
[2] stage
[3] stash
You can choose or quit [1,2,3,q]:
---
Best
Stephan
^ permalink raw reply
* Re: [RFC/PATCH v3 00/16] Add initial experimental external ODB support
From: Lars Schneider @ 2016-12-18 13:13 UTC (permalink / raw)
To: Christian Couder
Cc: git, Junio C Hamano, Jeff King, Nguyen Thai Ngoc Duy, Mike Hommey,
Eric Wong, Christian Couder
In-Reply-To: <CAP8UFD2uyq3Uf1co_BUKJX_eogdCDJ30KJZmQ1BQXNQ1dw=w3A@mail.gmail.com>
> On 13 Dec 2016, at 18:20, Christian Couder <christian.couder@gmail.com> wrote:
>
> On Sat, Dec 3, 2016 at 7:47 PM, Lars Schneider <larsxschneider@gmail.com> wrote:
>>
>>> On 30 Nov 2016, at 22:04, Christian Couder <christian.couder@gmail.com> wrote:
>>>
>>> Goal
>>> ~~~~
>>>
>>> Git can store its objects only in the form of loose objects in
>>> separate files or packed objects in a pack file.
>>>
>>> To be able to better handle some kind of objects, for example big
>>> blobs, it would be nice if Git could store its objects in other object
>>> databases (ODB).
>> ...
>>
>> Minor nit: I feel the term "other" could be more expressive. Plus
>> "database" might confuse people. What do you think about
>> "External Object Storage" or something?
>
> In the current Git code, "DB" is already used a lot. For example in
> cache.h there is:
> ...
I am not worried about Git core developers as I don't think they would
be confused by the term "DB". I wonder if it would make sense to have
a clearer "external name" for the average Git user (== non Git devs)
or if this would create just more confusion.
>>> * Transfer
>>>
>>> To tranfer information about the blobs stored in external ODB, some
>>> special refs, called "odb ref", similar as replace refs, are used.
>>>
>>> For now there should be one odb ref per blob. Each ref name should be
>>> refs/odbs/<odbname>/<sha1> where <sha1> is the sha1 of the blob stored
>>> in the external odb named <odbname>.
>>>
>>> These odb refs should all point to a blob that should be stored in the
>>> Git repository and contain information about the blob stored in the
>>> external odb. This information can be specific to the external odb.
>>> The repos can then share this information using commands like:
>>>
>>> `git fetch origin "refs/odbs/<odbname>/*:refs/odbs/<odbname>/*"`
>>
>> The "odbref" would point to a blob and the blob could contain anything,
>> right? E.g. it could contain an existing GitLFS pointer, right?
>>
>> version https://git-lfs.github.com/spec/v1
>> oid sha256:4d7a214614ab2935c943f9e0ff69d22eadbb8f32b1258daaa5e2ca24d17e2393
>> size 12345
>
> Yes, but I think that the sha1 should be added. So yes, it could
> easily be made compatible with git LFS.
What do you mean with "the sha1 should be added"? Do you suggest to add
sha1 to GitLFS?
>>> Future work
>>> ~~~~~~~~~~~
>>>
>>> I think that the odb refs don't prevent a regular fetch or push from
>>> wanting to send the objects that are managed by an external odb. So I
>>> am interested in suggestions about this problem. I will take a look at
>>> previous discussions and how other mechanisms (shallow clone, bundle
>>> v3, ...) handle this.
>>
>> If the ODB configuration is stored in the Git repo similar to
>> .gitmodules then every client that clones ODB references would be able
>> to resolve them, right?
>
> Yeah, but I am not sure that being able to resolve the odb refs will
> prevent the big blobs from being sent.
> With Git LFS, git doesn't know about the big blobs, only about the
> substituted files, but that is not the case in what I am doing.
I think the biggest problem in Git are huge blobs that are not in the
head revision. In the great majority of cases you don't need these blobs
but you always have to transfer them during clone. That's what GitLFS
is solving today and what I hope your protocol could solve better in
the future!
Cheers,
Lars
^ permalink raw reply
* [PATCH v1] t0021: fix flaky test
From: larsxschneider @ 2016-12-18 12:37 UTC (permalink / raw)
To: git; +Cc: ramsay, peff, gitster, Lars Schneider
In-Reply-To: <B3D96792-047D-4C91-8DCC-60C800B2861B@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>
t0021.15 creates files, adds them to the index, and commits them. All
this usually happens in a test run within the same second and Git cannot
know if the files have been changed between `add` and `commit`. Thus,
Git has to run the clean filter in both operations. Sometimes these
invocations spread over two different seconds and Git can infer that the
files were not changed between `add` and `commit` based on their
modification timestamp. The test would fail as it expects the filter
invocation. Remove this expectation to make the test stable.
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Notes:
Base Commit: f8bf8f2a7b (next)
Diff on Web: https://github.com/git/git/compare/f8bf8f2a7b...larsxschneider:9d88b66e03
Checkout: git fetch https://github.com/larsxschneider/git filter-process/fix-flaky-test-v1 && git checkout 9d88b66e03
t/t0021-conversion.sh | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 6f16983d3e..161f560446 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -377,22 +377,7 @@ test_expect_success PERL 'required process filter should filter data' '
EOF
test_cmp_count expected.log rot13-filter.log &&
- filter_git commit -m "test commit 2" &&
- cat >expected.log <<-EOF &&
- START
- init handshake complete
- IN: clean test.r $S [OK] -- OUT: $S . [OK]
- IN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]
- IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
- IN: clean testsubdir/test3 '\''sq'\'',\$x=.r $S3 [OK] -- OUT: $S3 . [OK]
- IN: clean test.r $S [OK] -- OUT: $S . [OK]
- IN: clean test2.r $S2 [OK] -- OUT: $S2 . [OK]
- IN: clean test4-empty.r 0 [OK] -- OUT: 0 [OK]
- IN: clean testsubdir/test3 '\''sq'\'',\$x=.r $S3 [OK] -- OUT: $S3 . [OK]
- STOP
- EOF
- test_cmp_count expected.log rot13-filter.log &&
-
+ git commit -m "test commit 2" &&
rm -f test2.r "testsubdir/test3 '\''sq'\'',\$x=.r" &&
filter_git checkout --quiet --no-progress . &&
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 1/5] doc: add documentation for OPT_STRING_LIST
From: Jacob Keller @ 2016-12-18 12:19 UTC (permalink / raw)
To: Philip Oakley, Jacob Keller, git; +Cc: Junio C Hamano
In-Reply-To: <67572777448E4DCE967BA079110A3487@PhilipOakley>
On December 17, 2016 3:56:19 AM PST, Philip Oakley <philipoakley@iee.org> wrote:
>From: "Jacob Keller" <jacob.e.keller@intel.com>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Commit c8ba16391655 ("parse-options: add OPT_STRING_LIST helper",
>> 2011-06-09) added the OPT_STRING_LIST as a way to accumulate a
>repeated
>> list of strings. However, this was not documented in the
>> api-parse-options documentation. Add documentation now so that future
>> developers may learn of its existence.
>>
>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
>> ---
>> Documentation/technical/api-parse-options.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/technical/api-parse-options.txt
>> b/Documentation/technical/api-parse-options.txt
>> index 27bd701c0d68..92791740aa64 100644
>> --- a/Documentation/technical/api-parse-options.txt
>> +++ b/Documentation/technical/api-parse-options.txt
>> @@ -168,6 +168,11 @@ There are some macros to easily define options:
>> Introduce an option with string argument.
>> The string argument is put into `str_var`.
>>
>> +`OPT_STRING_LIST(short long, &list, arg_str, description)`::
>
>should there be an extra comma between 'short long' in a similar manner
>to
>the OPT_INTEGER argument list below?
>
>
You are indeed correct sir. I will fix this up.
Thanks,
Jake
^ permalink raw reply
* Suggestion for the "Did you mean this?" feature
From: Kaartic Sivaraam @ 2016-12-18 12:18 UTC (permalink / raw)
To: git
Hello all,
I have found the "Did you mean this?" feature of git as a very good
feature. I thought it would be even better if it took a step toward by
asking for a prompt when there was only one alternative to the command
that was entered.
E.g.
> unique@unique-pc:~$ git hepl
> git: 'hepl' is not a git command. See 'git --help'.
>
> Did you mean this?
> help
> [yes/No] : y
> usage: git [--version] [--help] [-C <path>] [-c name=value]
> [--exec-path[=<path>]] [--html-path] [--man-path] [--info-
> path]
> ....
This would make it even better for the user as it would avoid having to
correct the mistake long commands that had only a single error
(considering history feature is enabled).
Is this is a good idea ?
--
Regards,
Kaartic
^ permalink raw reply
* [PATCH v2] git-p4: Fix multi-path changelist empty commits
From: George Vanburgh @ 2016-12-17 22:11 UTC (permalink / raw)
To: git
In-Reply-To: <01020159037a8995-2d1da9d4-4a27-4b98-818b-432fc0ad8a52-000000@eu-west-1.amazonses.com>
From: George Vanburgh <gvanburgh@bloomberg.net>
When importing from multiple perforce paths - we may attempt to import
a changelist that contains files from two (or more) of these depot
paths. Currently, this results in multiple git commits - one
containing the changes, and the other(s) as empty commit(s). This
behavior was introduced in commit 1f90a64
("git-p4: reduce number of server queries for fetches", 2015-12-19).
Reproduction Steps:
1. Have a git repo cloned from a perforce repo using multiple depot
paths (e.g. //depot/foo and //depot/bar).
2. Submit a single change to the perforce repo that makes changes in
both //depot/foo and //depot/bar.
3. Run "git p4 sync" to sync the change from #2.
Change is synced as multiple commits, one for each depot path that was
affected.
Using a set, instead of a list inside p4ChangesForPaths() ensures that
each changelist is unique to the returned list, and therefore only a
single commit is generated for each changelist.
Reported-by: James Farwell <jfarwell@vmware.com>
Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net>
---
git-p4.py | 4 ++--
t/t9800-git-p4-basic.sh | 22 +++++++++++++++++++++-
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index fd5ca52..6307bc8 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -822,7 +822,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
die("cannot use --changes-block-size with non-numeric revisions")
block_size = None
- changes = []
+ changes = set()
# Retrieve changes a block at a time, to prevent running
# into a MaxResults/MaxScanRows error from the server.
@@ -841,7 +841,7 @@ def p4ChangesForPaths(depotPaths, changeRange, requestedBlockSize):
# Insert changes in chronological order
for line in reversed(p4_read_pipe_lines(cmd)):
- changes.append(int(line.split(" ")[1]))
+ changes.add(int(line.split(" ")[1]))
if not block_size:
break
diff --git a/t/t9800-git-p4-basic.sh b/t/t9800-git-p4-basic.sh
index 0730f18..4d93522 100755
--- a/t/t9800-git-p4-basic.sh
+++ b/t/t9800-git-p4-basic.sh
@@ -131,6 +131,26 @@ test_expect_success 'clone two dirs, @all, conflicting files' '
)
'
+test_expect_success 'clone two dirs, each edited by submit, single git commit' '
+ (
+ cd "$cli" &&
+ echo sub1/f4 >sub1/f4 &&
+ p4 add sub1/f4 &&
+ echo sub2/f4 >sub2/f4 &&
+ p4 add sub2/f4 &&
+ p4 submit -d "sub1/f4 and sub2/f4"
+ ) &&
+ git p4 clone --dest="$git" //depot/sub1@all //depot/sub2@all &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git ls-files >lines &&
+ test_line_count = 4 lines &&
+ git log --oneline p4/master >lines &&
+ test_line_count = 5 lines
+ )
+'
+
revision_ranges="2000/01/01,#head \
1,2080/01/01 \
2000/01/01,2080/01/01 \
@@ -147,7 +167,7 @@ test_expect_success 'clone using non-numeric revision ranges' '
(
cd "$git" &&
git ls-files >lines &&
- test_line_count = 6 lines
+ test_line_count = 8 lines
)
done
'
--
https://github.com/git/git/pull/311
^ permalink raw reply related
* Re: [PATCH v2 01/34] sequencer: support a new action: 'interactive rebase'
From: Stephan Beyer @ 2016-12-17 20:23 UTC (permalink / raw)
To: Junio C Hamano, Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqy3ziwbpk.fsf@gitster.mtv.corp.google.com>
Hi,
On 12/14/2016 08:29 PM, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>> -/* We will introduce the 'interactive rebase' mode later */
>> static inline int is_rebase_i(const struct replay_opts *opts)
>> {
>> - return 0;
>> + return opts->action == REPLAY_INTERACTIVE_REBASE;
>> }
>>
>> static const char *get_dir(const struct replay_opts *opts)
>> {
>> + if (is_rebase_i(opts))
>> + return rebase_path();
>> return git_path_seq_dir();
>> }
>
> This obviously makes the assumption made by 39784cd362 ("sequencer:
> remove useless get_dir() function", 2016-12-07) invalid, where the
> "remove useless" thought that the callers of this function wants a
> single answer that does not depend on opts.
>
> I'll revert that commit from the sb/sequencer-abort-safety topic (as
> the topic is in 'next' already) to keep this one. Please holler if
> that is a mistaken decision.
It seems to be the right decision if one wants to keep the internal-path
backwards-compatibility of "git rebase -i" (and it seems that Dscho
wants to keep it).
However, maintaining more than one directory (like "sequencer" for
sequencer state and "rebase-merge" for rebase todo and log) can cause
the necessity to be even more careful when hacking on sequencer... For
example, the cleanup code must delete both of them, not only one of them.
Hence, I think that it's a wiser choice to keep one directory for
sequencer state *and* additional files.
If we (a) save everything in "sequencer", we break the internal-path
backwards-compatbility but we can get rid of get_dir()...
If we (b) save everything in "rebase-merge" when using rebase -i and
save everything in "sequencer" for pick/revert, we are 100%
backwards-compatible, but we have to change every occurrence of
git_path_seq_dir() to get_dir() and the GIT_PATH_FUNC definitions of
git_path_{todo,opts,head}_file must be replaced by definitions using
get_dir().
Best
Stephan
^ permalink raw reply
* Re: [PATCH v15 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
From: Stephan Beyer @ 2016-12-17 19:55 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPMwviof5jNvQnxFZYdw324XpbLSQ9mzEQjrCW-K4A+GCA@mail.gmail.com>
Hi Pranit,
On 12/16/2016 08:35 PM, Pranit Bauva wrote:
> On Thu, Nov 17, 2016 at 5:17 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
>> On 10/14/2016 04:14 PM, Pranit Bauva wrote:
>>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>>> index d84ba86..c542e8b 100644
>>> --- a/builtin/bisect--helper.c
>>> +++ b/builtin/bisect--helper.c
>>> @@ -123,13 +123,40 @@ static int bisect_reset(const char *commit)
>>> return bisect_clean_state();
>>> }
>>>
>>> +static int is_expected_rev(const char *expected_hex)
>>> +{
>>> + struct strbuf actual_hex = STRBUF_INIT;
>>> + int res = 0;
>>> + if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), 0) >= 40) {
>>> + strbuf_trim(&actual_hex);
>>> + res = !strcmp(actual_hex.buf, expected_hex);
>>> + }
>>> + strbuf_release(&actual_hex);
>>> + return res;
>>> +}
>>
>> I am not sure it does what it should.
>>
>> I would expect the following behavior from this function:
>> - file does not exist (or is "broken") => return 0
>> - actual_hex != expected_hex => return 0
>> - otherwise return 1
>>
>> If I am not wrong, the code does the following instead:
>> - file does not exist (or is "broken") => return 0
>> - actual_hex != expected_hex => return 1
>> - otherwise => return 0
>
> It seems that I didn't carefully see what the shell code is (or
> apparently did a mistake in understanding it ;)). I think the C
> version does exactly what the shell version does. Can you confirm it
> once again, please?
I check again...
The shell code does the following:
- file does not exist or is not a regular file => return false
- actual_hex != expected_hex => return false
- otherwise => return true
(false means a value != 0, true means 0)
Your code does the following:
- file cannot be read or is broken => return 0
- actual_hex != expected_hex => return 0
- otherwise => return 1
So you are very right, it seems I had a weird thinko (I probably missed
the "!" in front of strcmp or something) :)
Sorry for bothering,
Stephan
^ permalink raw reply
* [PATCH] mailinfo.c: move side-effects outside of assert
From: Kyle J. McKay @ 2016-12-17 19:54 UTC (permalink / raw)
To: Jonathan Tan, Junio C Hamano; +Cc: Git mailing list
Since 6b4b013f18 (mailinfo: handle in-body header continuations,
2016-09-20, v2.11.0) mailinfo.c has contained new code with an
assert of the form:
assert(call_a_function(...))
The function in question, check_header, has side effects. This
means that when NDEBUG is defined during a release build the
function call is omitted entirely, the side effects do not
take place and tests (fortunately) start failing.
Move the function call outside of the assert and assert on
the result of the function call instead so that the code
still works properly in a release build and passes the tests.
Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
---
Notes:
Please include this PATCH in 2.11.x maint
mailinfo.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/mailinfo.c b/mailinfo.c
index 2fb3877e..47442fb5 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -708,9 +708,12 @@ static int is_scissors_line(const char *line)
static void flush_inbody_header_accum(struct mailinfo *mi)
{
+ int okay;
+
if (!mi->inbody_header_accum.len)
return;
- assert(check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0));
+ okay = check_header(mi, &mi->inbody_header_accum, mi->s_hdr_data, 0);
+ assert(okay);
strbuf_reset(&mi->inbody_header_accum);
}
---
^ permalink raw reply related
* Re: [PATCH v15 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
From: Stephan Beyer @ 2016-12-17 19:42 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPO2WgBjOnmvu1VOiz3PMYYx2mxircCWk+BWxmuunC=VQA@mail.gmail.com>
Hi Pranit,
On 12/16/2016 08:00 PM, Pranit Bauva wrote:
> On Wed, Dec 7, 2016 at 1:03 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>> I don't understand why the return value is int and not void. To avoid a
>>> "return 0;" line when calling this function?
>>
>> Initially I thought I would be using the return value but now I
>> realize that it is meaningless to do so. Using void seems better. :)
>
> I just recollected when I was creating the next iteration of this
> series that I will need that int.
>
>>>> + case CHECK_EXPECTED_REVS:
>>>> + return check_expected_revs(argv, argc);
>
> See this.
This does not show that you need the "int", it just shows that you use
the return value of the function. But this return value is (in the
original shell code as well as in your v15) always 0. That is a sign
that the "void" return value makes more sense. Of course, then the line
above must be changed to
+ case CHECK_EXPECTED_REVS:
+ check_expected_revs(argv, argc);
+ return 0;
By the way, it also seems that the original function name
"check_expected_revs" was not the best choice (because the function
always returns 0, but the "check" implies that some semantically true or
false is returned).
On the other hand, it might also be useful (I cannot tell) to return
different values in check_expected_revs() — to signal the caller that
something changed or something did not change — but then ignore the
return value.
Best
Stephan
^ permalink raw reply
* Re: test failure
From: Lars Schneider @ 2016-12-17 16:11 UTC (permalink / raw)
To: Ramsay Jones, Jeff King; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <50C75781-FE3B-410F-9866-63342607707B@gmail.com>
> On 17 Dec 2016, at 15:28, Lars Schneider <larsxschneider@gmail.com> wrote:
>
>
>> On 16 Dec 2016, at 21:32, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>>
>> Hi Lars,
>>
>> For the last two days, I've noticed t0021.15 on the 'pu' branch has been failing intermittently (well it fails with: 'make test >ptest-out', but
>> when run by hand, it fails only say 1-in-6, 1-in-18, etc.).
>>
>> [yes, it's a bit strange; this hasn't changed in a couple of weeks!]
>>
>> I don't have time to investigate further tonight and, since I had not
>> heard anyone else complain, I thought I should let you know.
>>
>> See below for the output from a failing run. [Note: this is on Linux
>> Mint 18, tonight's pu branch @7c7984401].
>
> Thanks Ramsay!
>
> I was able to reproduce the problem with this test:
>
> test_expect_success 'ramsay-report' '
> test_config_global filter.protocol.clean cat &&
> git init &&
> echo "*.r filter=protocol" >.gitattributes &&
> echo "bla" >test.r &&
> git add . &&
> GIT_TRACE=1 git commit -m "test commit 2" > trace 2>&1 &&
> grep "run_command" trace
> '
>
> It looks like as if Git occasionally forgets to run the clean filter.
> I bisected the problem and I think the problem starts with "diff: do not
> reuse worktree files that need "clean" conversion" (06dec439a3) which
> definitively sounds related.
>
> Back in June I reported that Git invokes the clean process 4 times if a
> single file is added. Peff took a closer look and suggested the patch
> mentioned above to remove one unnecessary invocation. I re-read his comments
> and everything sounds still reasonable to me:
> http://public-inbox.org/git/1469134747-26785-1-git-send-email-larsxschneider@gmail.com/#t
>
> Does anyone have a clue what is going on?
> I keep digging...
Ugh. I stopped coding, started cleaning the house, and it hit me:
"git commit" shouldn't call the filter anyways. I suspect it is called in
my tests because I add and commit the file to the index right after its
creation. All this usually happens within 1 second and therefore Git cannot
know if the file was modified between "add" and "commit". That's why it needs
to run "clean" again.
I will adjust the tests.
Cheers,
Lars
^ permalink raw reply
* Re: [PATCH v2 00/21] Add configuration options for split-index
From: Christian Couder @ 2016-12-17 15:35 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
> The previous versions were:
>
> RFC: https://github.com/chriscool/git/commits/config-split-index7
> v1: https://github.com/chriscool/git/commits/config-split-index72
The diff since v1 is:
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8fbef25cb1..52a3cac4ff 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2782,9 +2782,9 @@ splitIndex.sharedIndexExpire::
index file is created. The value "now" expires all entries
immediately, and "never" suppresses expiration altogether.
The default value is "one.week.ago".
- Note that each time a new split-index file is created, the
- mtime of the related shared index file is updated to the
- current time.
+ Note that each time a split index based on a shared index file
+ is either created or read from, the mtime of the shared index
+ file is updated to the current time.
See linkgit:git-update-index[1].
status.relativePaths::
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 635d1574b2..46c953b2f2 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -407,10 +407,14 @@ specified by the splitIndex.maxPercentChange
config variable (see
linkgit:git-config[1]).
Each time a new shared index file is created, the old shared index
-files are deleted if they are older than what is specified by the
-splitIndex.sharedIndexExpire config variable (see
+files are deleted if their mtime is older than what is specified by
+the splitIndex.sharedIndexExpire config variable (see
linkgit:git-config[1]).
+To avoid deleting a shared index file that is still used, its mtime is
+updated to the current time everytime a new split index based on the
+shared index file is either created or read from.
+
Untracked cache
---------------
diff --git a/builtin/gc.c b/builtin/gc.c
index c1e9602892..1e40d45aa2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -100,8 +100,8 @@ static void gc_config(void)
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);
- git_config_get_date_string("gc.pruneexpire", &prune_expire);
- git_config_get_date_string("gc.worktreepruneexpire",
&prune_worktrees_expire);
+ git_config_get_expiry("gc.pruneexpire", &prune_expire);
+ git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
git_config(git_default_config, NULL);
}
diff --git a/builtin/update-index.c b/builtin/update-index.c
index a14dbf2612..dc1fd0d44d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1099,18 +1099,18 @@ int cmd_update_index(int argc, const char
**argv, const char *prefix)
if (split_index > 0) {
if (git_config_get_split_index() == 0)
- warning("core.splitIndex is set to false; "
- "remove or change it, if you really want to "
- "enable split index");
+ warning(_("core.splitIndex is set to false; "
+ "remove or change it, if you really want to "
+ "enable split index"));
if (the_index.split_index)
the_index.cache_changed |= SPLIT_INDEX_ORDERED;
else
add_split_index(&the_index);
} else if (!split_index) {
if (git_config_get_split_index() == 1)
- warning("core.splitIndex is set to true; "
- "remove or change it, if you really want to "
- "disable split index");
+ warning(_("core.splitIndex is set to true; "
+ "remove or change it, if you really want to "
+ "disable split index"));
remove_split_index(&the_index);
}
diff --git a/cache.h b/cache.h
index 8e26aaf05e..279415afbd 100644
--- a/cache.h
+++ b/cache.h
@@ -1823,11 +1823,13 @@ extern int git_config_get_bool(const char
*key, int *dest);
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);
-extern int git_config_get_date_string(const char *key, const char **output);
extern int git_config_get_untracked_cache(void);
extern int git_config_get_split_index(void);
extern int git_config_get_max_percent_split_change(void);
+/* This dies if the configured or default date is in the future */
+extern int git_config_get_expiry(const char *key, const char **output);
+
/*
* This is a hack for test programs like test-dump-untracked-cache to
* ensure that they do not modify the untracked cache when reading it.
diff --git a/config.c b/config.c
index f88c61bb30..5c52cefd78 100644
--- a/config.c
+++ b/config.c
@@ -1685,7 +1685,7 @@ int git_config_get_pathname(const char *key,
const char **dest)
return ret;
}
-int git_config_get_date_string(const char *key, const char **output)
+int git_config_get_expiry(const char *key, const char **output)
{
int ret = git_config_get_string_const(key, output);
if (ret)
@@ -1714,8 +1714,8 @@ int git_config_get_untracked_cache(void)
if (!strcasecmp(v, "keep"))
return -1;
- error("unknown core.untrackedCache value '%s'; "
- "using 'keep' default value", v);
+ error(_("unknown core.untrackedCache value '%s'; "
+ "using 'keep' default value"), v);
return -1;
}
@@ -1740,9 +1740,8 @@ int git_config_get_max_percent_split_change(void)
if (0 <= val && val <= 100)
return val;
- error("splitindex.maxpercentchange value '%d' "
- "should be between 0 and 100", val);
- return -1;
+ return error(_("splitIndex.maxPercentChange value '%d' "
+ "should be between 0 and 100"), val);
}
return -1; /* default value */
diff --git a/read-cache.c b/read-cache.c
index 9727efcb5b..35377f0a3e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1685,10 +1685,25 @@ int do_read_index(struct index_state *istate,
const char *path, int must_exist)
die("index file corrupt");
}
+/*
+ * Signal that the shared index is used by updating its mtime.
+ *
+ * This way, shared index can be removed if they have not been used
+ * for some time. It's ok to fail to update the mtime if we are on a
+ * read only file system.
+ */
+void freshen_shared_index(char *base_sha1_hex)
+{
+ const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
+ check_and_freshen_file(shared_index, 1);
+}
+
int read_index_from(struct index_state *istate, const char *path)
{
struct split_index *split_index;
int ret;
+ char *base_sha1_hex;
+ const char *base_path;
/* istate->initialized covers both .git/index and .git/sharedindex.xxx */
if (istate->initialized)
@@ -1706,15 +1721,16 @@ int read_index_from(struct index_state
*istate, const char *path)
discard_index(split_index->base);
else
split_index->base = xcalloc(1, sizeof(*split_index->base));
- ret = do_read_index(split_index->base,
- git_path("sharedindex.%s",
- sha1_to_hex(split_index->base_sha1)), 1);
+
+ base_sha1_hex = sha1_to_hex(split_index->base_sha1);
+ base_path = git_path("sharedindex.%s", base_sha1_hex);
+ ret = do_read_index(split_index->base, base_path, 1);
if (hashcmp(split_index->base_sha1, split_index->base->sha1))
die("broken index, expect %s in %s, got %s",
- sha1_to_hex(split_index->base_sha1),
- git_path("sharedindex.%s",
- sha1_to_hex(split_index->base_sha1)),
+ base_sha1_hex, base_path,
sha1_to_hex(split_index->base->sha1));
+
+ freshen_shared_index(base_sha1_hex);
merge_base_index(istate);
post_read_index_from(istate);
return ret;
@@ -2205,8 +2221,8 @@ static unsigned long get_shared_index_expire_date(void)
static int shared_index_expire_date_prepared;
if (!shared_index_expire_date_prepared) {
- git_config_get_date_string("splitindex.sharedindexexpire",
- &shared_index_expire);
+ git_config_get_expiry("splitindex.sharedindexexpire",
+ &shared_index_expire);
shared_index_expire_date = approxidate(shared_index_expire);
shared_index_expire_date_prepared = 1;
}
@@ -2225,22 +2241,20 @@ static int can_delete_shared_index(const char
*shared_sha1_hex)
if (!expiration)
return 0;
if (stat(shared_index, &st))
- return error_errno("could not stat '%s", shared_index);
+ return error_errno(_("could not stat '%s"), shared_index);
if (st.st_mtime > expiration)
return 0;
return 1;
}
-static void clean_shared_index_files(const char *current_hex)
+static int clean_shared_index_files(const char *current_hex)
{
struct dirent *de;
DIR *dir = opendir(get_git_dir());
- if (!dir) {
- error_errno("unable to open git dir: %s", get_git_dir());
- return;
- }
+ if (!dir)
+ return error_errno(_("unable to open git dir: %s"), get_git_dir());
while ((de = readdir(dir)) != NULL) {
const char *sha1_hex;
@@ -2250,9 +2264,11 @@ static void clean_shared_index_files(const char
*current_hex)
continue;
if (can_delete_shared_index(sha1_hex) > 0 &&
unlink(git_path("%s", de->d_name)))
- error_errno("unable to unlink: %s", git_path("%s", de->d_name));
+ error_errno(_("unable to unlink: %s"), git_path("%s", de->d_name));
}
closedir(dir);
+
+ return 0;
}
static struct tempfile temporary_sharedindex;
@@ -2286,7 +2302,7 @@ static int write_shared_index(struct index_state *istate,
static const int default_max_percent_split_change = 20;
-int too_many_not_shared_entries(struct index_state *istate)
+static int too_many_not_shared_entries(struct index_state *istate)
{
int i, not_shared = 0;
int max_split = git_config_get_max_percent_split_change();
@@ -2331,17 +2347,14 @@ int write_locked_index(struct index_state
*istate, struct lock_file *lock,
if ((v & 15) < 6)
istate->cache_changed |= SPLIT_INDEX_ORDERED;
}
- if (istate->cache_changed & SPLIT_INDEX_ORDERED ||
- too_many_not_shared_entries(istate)) {
+ if (too_many_not_shared_entries(istate))
+ istate->cache_changed |= SPLIT_INDEX_ORDERED;
+ if (istate->cache_changed & SPLIT_INDEX_ORDERED) {
int ret = write_shared_index(istate, lock, flags);
if (ret)
return ret;
} else {
- /* Signal that the shared index is used */
- const char *shared_index = git_path("sharedindex.%s",
- sha1_to_hex(si->base_sha1));
- if (!check_and_freshen_file(shared_index, 1))
- warning("could not freshen '%s'", shared_index);
+ freshen_shared_index(sha1_to_hex(si->base_sha1));
}
return write_split_index(istate, lock, flags);
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index f448fc13cd..800f84a593 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -313,17 +313,17 @@ EOF
test_expect_success 'shared index files expire after 7 days by default' '
: >ten &&
git update-index --add ten &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
just_under_7_days_ago=$((1-7*86400)) &&
test-chmtime =$just_under_7_days_ago .git/sharedindex.* &&
: >eleven &&
git update-index --add eleven &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
just_over_7_days_ago=$((-1-7*86400)) &&
test-chmtime =$just_over_7_days_ago .git/sharedindex.* &&
: >twelve &&
git update-index --add twelve &&
- test $(ls .git/sharedindex.* | wc -l) = 1
+ test $(ls .git/sharedindex.* | wc -l) -le 2
'
test_expect_success 'check splitIndex.sharedIndexExpire set to 8 days' '
@@ -331,12 +331,12 @@ test_expect_success 'check
splitIndex.sharedIndexExpire set to 8 days' '
test-chmtime =$just_over_7_days_ago .git/sharedindex.* &&
: >thirteen &&
git update-index --add thirteen &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
just_over_8_days_ago=$((-1-8*86400)) &&
test-chmtime =$just_over_8_days_ago .git/sharedindex.* &&
: >fourteen &&
git update-index --add fourteen &&
- test $(ls .git/sharedindex.* | wc -l) = 1
+ test $(ls .git/sharedindex.* | wc -l) -le 2
'
test_expect_success 'check splitIndex.sharedIndexExpire set to
"never" and "now"' '
@@ -345,13 +345,13 @@ test_expect_success 'check
splitIndex.sharedIndexExpire set to "never" and "now"
test-chmtime =$just_10_years_ago .git/sharedindex.* &&
: >fifteen &&
git update-index --add fifteen &&
- test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 2 &&
git config splitIndex.sharedIndexExpire now &&
just_1_second_ago=-1 &&
test-chmtime =$just_1_second_ago .git/sharedindex.* &&
: >sixteen &&
git update-index --add sixteen &&
- test $(ls .git/sharedindex.* | wc -l) = 1
+ test $(ls .git/sharedindex.* | wc -l) -le 2
'
test_done
^ permalink raw reply related
* [PATCH v2 07/21] Documentation/config: add information for core.splitIndex
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a0ab66aae7..dc44d8a417 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -331,6 +331,10 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
+core.splitIndex::
+ If true, the split-index feature of the index will be used.
+ See linkgit:git-update-index[1]. False by default.
+
core.untrackedCache::
Determines what to do about the untracked cache feature of the
index. It will be kept, if this variable is unset or set to
--
2.11.0.49.g2414764.dirty
^ permalink raw reply related
* [PATCH v2 03/21] split-index: add {add,remove}_split_index() functions
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
Also use the functions in cmd_update_index() in
builtin/update-index.c.
These functions will be used in a following commit to tweak
our use of the split-index feature depending on the setting
of a configuration variable.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/update-index.c | 18 ++++++------------
split-index.c | 22 ++++++++++++++++++++++
split-index.h | 2 ++
3 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index f3f07e7f1c..b75ea037dd 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1098,18 +1098,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
}
if (split_index > 0) {
- init_split_index(&the_index);
- the_index.cache_changed |= SPLIT_INDEX_ORDERED;
- } else if (!split_index && the_index.split_index) {
- /*
- * can't discard_split_index(&the_index); because that
- * will destroy split_index->base->cache[], which may
- * be shared with the_index.cache[]. So yeah we're
- * leaking a bit here.
- */
- the_index.split_index = NULL;
- the_index.cache_changed |= SOMETHING_CHANGED;
- }
+ if (the_index.split_index)
+ the_index.cache_changed |= SPLIT_INDEX_ORDERED;
+ else
+ add_split_index(&the_index);
+ } else if (!split_index)
+ remove_split_index(&the_index);
switch (untracked_cache) {
case UC_UNSPECIFIED:
diff --git a/split-index.c b/split-index.c
index 615f4cac05..f519e60f87 100644
--- a/split-index.c
+++ b/split-index.c
@@ -317,3 +317,25 @@ void replace_index_entry_in_base(struct index_state *istate,
istate->split_index->base->cache[new->index - 1] = new;
}
}
+
+void add_split_index(struct index_state *istate)
+{
+ if (!istate->split_index) {
+ init_split_index(istate);
+ istate->cache_changed |= SPLIT_INDEX_ORDERED;
+ }
+}
+
+void remove_split_index(struct index_state *istate)
+{
+ if (istate->split_index) {
+ /*
+ * can't discard_split_index(&the_index); because that
+ * will destroy split_index->base->cache[], which may
+ * be shared with the_index.cache[]. So yeah we're
+ * leaking a bit here.
+ */
+ istate->split_index = NULL;
+ istate->cache_changed |= SOMETHING_CHANGED;
+ }
+}
diff --git a/split-index.h b/split-index.h
index c1324f521a..df91c1bda8 100644
--- a/split-index.h
+++ b/split-index.h
@@ -31,5 +31,7 @@ void merge_base_index(struct index_state *istate);
void prepare_to_write_split_index(struct index_state *istate);
void finish_writing_split_index(struct index_state *istate);
void discard_split_index(struct index_state *istate);
+void add_split_index(struct index_state *istate);
+void remove_split_index(struct index_state *istate);
#endif
--
2.11.0.49.g2414764.dirty
^ permalink raw reply related
* [PATCH v2 04/21] read-cache: add and then use tweak_split_index()
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
This will make us use the split-index feature or not depending
on the value of the "core.splitIndex" config variable.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index db5d910642..79aae6bd20 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1569,10 +1569,27 @@ static void tweak_untracked_cache(struct index_state *istate)
}
}
+static void tweak_split_index(struct index_state *istate)
+{
+ switch (git_config_get_split_index()) {
+ case -1: /* unset: do nothing */
+ break;
+ case 0: /* false */
+ remove_split_index(istate);
+ break;
+ case 1: /* true */
+ add_split_index(istate);
+ break;
+ default: /* unknown value: do nothing */
+ break;
+ }
+}
+
static void post_read_index_from(struct index_state *istate)
{
check_ce_order(istate);
tweak_untracked_cache(istate);
+ tweak_split_index(istate);
}
/* remember to discard_cache() before reading a different cache! */
--
2.11.0.49.g2414764.dirty
^ permalink raw reply related
* [PATCH v2 10/21] read-cache: regenerate shared index if necessary
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
When writing a new split-index and there is a big number of cache
entries in the split-index compared to the shared index, it is a
good idea to regenerate the shared index.
By default when the ratio reaches 20%, we will push back all
the entries from the split-index into a new shared index file
instead of just creating a new split-index file.
The threshold can be configured using the
"splitIndex.maxPercentChange" config variable.
We need to adjust the existing tests in t1700 by setting
"splitIndex.maxPercentChange" to 100 at the beginning of t1700,
as the existing tests are assuming that the shared index is
regenerated only when `git update-index --split-index` is used.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 32 ++++++++++++++++++++++++++++++++
t/t1700-split-index.sh | 1 +
2 files changed, 33 insertions(+)
diff --git a/read-cache.c b/read-cache.c
index 79aae6bd20..280b01bd6c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2223,6 +2223,36 @@ static int write_shared_index(struct index_state *istate,
return ret;
}
+static const int default_max_percent_split_change = 20;
+
+static int too_many_not_shared_entries(struct index_state *istate)
+{
+ int i, not_shared = 0;
+ int max_split = git_config_get_max_percent_split_change();
+
+ switch (max_split) {
+ case -1:
+ /* not or badly configured: use the default value */
+ max_split = default_max_percent_split_change;
+ break;
+ case 0:
+ return 1; /* 0% means always write a new shared index */
+ case 100:
+ return 0; /* 100% means never write a new shared index */
+ default:
+ ; /* do nothing: just use the configured value */
+ }
+
+ /* Count not shared entries */
+ for (i = 0; i < istate->cache_nr; i++) {
+ struct cache_entry *ce = istate->cache[i];
+ if (!ce->index)
+ not_shared++;
+ }
+
+ return istate->cache_nr * max_split < not_shared * 100;
+}
+
int write_locked_index(struct index_state *istate, struct lock_file *lock,
unsigned flags)
{
@@ -2240,6 +2270,8 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
if ((v & 15) < 6)
istate->cache_changed |= SPLIT_INDEX_ORDERED;
}
+ if (too_many_not_shared_entries(istate))
+ istate->cache_changed |= SPLIT_INDEX_ORDERED;
if (istate->cache_changed & SPLIT_INDEX_ORDERED) {
int ret = write_shared_index(istate, lock, flags);
if (ret)
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index db8c39f446..507a1dd1ad 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -8,6 +8,7 @@ test_description='split index mode tests'
sane_unset GIT_TEST_SPLIT_INDEX
test_expect_success 'enable split index' '
+ git config splitIndex.maxPercentChange 100 &&
git update-index --split-index &&
test-dump-split-index .git/index >actual &&
indexversion=$(test-index-version <.git/index) &&
--
2.11.0.49.g2414764.dirty
^ permalink raw reply related
* [PATCH v2 15/21] config: add git_config_get_expiry() from gc.c
From: Christian Couder @ 2016-12-17 14:55 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20161217145547.11748-1-chriscool@tuxfamily.org>
This function will be used in a following commit to get the expiration
time of the shared index files from the config, and it is generic
enough to be put in "config.c".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin/gc.c | 15 ++-------------
cache.h | 3 +++
config.c | 13 +++++++++++++
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 069950d0b4..1e40d45aa2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -62,17 +62,6 @@ static void report_pack_garbage(unsigned seen_bits, const char *path)
string_list_append(&pack_garbage, path);
}
-static void git_config_date_string(const char *key, const char **output)
-{
- if (git_config_get_string_const(key, output))
- return;
- if (strcmp(*output, "now")) {
- unsigned long now = approxidate("now");
- if (approxidate(*output) >= now)
- git_die_config(key, _("Invalid %s: '%s'"), key, *output);
- }
-}
-
static void process_log_file(void)
{
struct stat st;
@@ -111,8 +100,8 @@ static void gc_config(void)
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);
- git_config_date_string("gc.pruneexpire", &prune_expire);
- git_config_date_string("gc.worktreepruneexpire", &prune_worktrees_expire);
+ git_config_get_expiry("gc.pruneexpire", &prune_expire);
+ git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
git_config(git_default_config, NULL);
}
diff --git a/cache.h b/cache.h
index f442f28189..279415afbd 100644
--- a/cache.h
+++ b/cache.h
@@ -1827,6 +1827,9 @@ extern int git_config_get_untracked_cache(void);
extern int git_config_get_split_index(void);
extern int git_config_get_max_percent_split_change(void);
+/* This dies if the configured or default date is in the future */
+extern int git_config_get_expiry(const char *key, const char **output);
+
/*
* This is a hack for test programs like test-dump-untracked-cache to
* ensure that they do not modify the untracked cache when reading it.
diff --git a/config.c b/config.c
index 3e96c223f5..5c52cefd78 100644
--- a/config.c
+++ b/config.c
@@ -1685,6 +1685,19 @@ int git_config_get_pathname(const char *key, const char **dest)
return ret;
}
+int git_config_get_expiry(const char *key, const char **output)
+{
+ int ret = git_config_get_string_const(key, output);
+ if (ret)
+ return ret;
+ if (strcmp(*output, "now")) {
+ unsigned long now = approxidate("now");
+ if (approxidate(*output) >= now)
+ git_die_config(key, _("Invalid %s: '%s'"), key, *output);
+ }
+ return ret;
+}
+
int git_config_get_untracked_cache(void)
{
int val = -1;
--
2.11.0.49.g2414764.dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox