Re: [PATCH 1/3] test-lib: Document short options in t/README
On 24/03/14 08:49, Ilya Bobyr wrote: > Most arguments that could be provided to a test have short forms. > Unless documented the only way to learn then is to read the code. > > Signed-off-by: Ilya Bobyr > --- > t/README | 10 +- > 1 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/t/README b/t/README > index caeeb9d..ccb5989 100644 > --- a/t/README > +++ b/t/README > @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and > --immediate > (or -i) command line argument to the test, or by setting GIT_TEST_OPTS > appropriately before running "make". > > ---verbose:: > +-v,--verbose:: OK > This makes the test more verbose. Specifically, the > command being run and their output if any are also > output. > @@ -81,7 +81,7 @@ appropriately before running "make". > numbers matching . The number matched against is > simply the running count of the test within the file. > > ---debug:: > +-d,--debug:: > This may help the person who is developing a new test. > It causes the command defined with test_debug to run. > The "trash" directory (used to store all temporary data > @@ -89,18 +89,18 @@ appropriately before running "make". > failed tests so that you can inspect its contents after > the test finished. > > ---immediate:: > +-i,--immediate:: > This causes the test to immediately exit upon the first > failed test. Cleanup commands requested with > test_when_finished are not executed if the test failed, > in order to keep the state for inspection by the tester > to diagnose the bug. > > ---long-tests:: > +-l,--long-tests:: > This causes additional long-running tests to be run (where > available), for more exhaustive testing. > > ---valgrind=:: > +-v,--valgrind=:: The -v short option is taken, above ... :-P > Execute all Git binaries under valgrind tool and exit > with status 126 on errors (just like regular tests, this will > only stop the test script when running under -i). > ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 00/11] Add interpret-trailers builtin
On 04/03/14 19:47, Christian Couder wrote:
> This patch series implements a new command:
>
> git interpret-trailers
>
[snip]
Minor problem: this series causes sparse to complain, thus:
trailer.c:642:6: warning: symbol 'process_trailers' was not \
declared. Should it be static?
The following patch, on top of the 'pu' branch, fixes it:
--- >8 ---
Subject: [PATCH] trailer.c: suppress sparse warning
Check that the public interface, as declared in the trailer.h header
file, is consistent with the actual implementation. Add an #include
of the header file into the implementation file.
Noticed by sparse ("'process_trailers' was not declared. Should it
be static?").
Signed-off-by: Ramsay Jones
---
trailer.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/trailer.c b/trailer.c
index b5de616..95d5874 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "run-command.h"
#include "argv-array.h"
+#include "trailer.h"
/*
* Copyright (c) 2013 Christian Couder
*/
--
1.9.0
--- 8< ---
However, for this to work, in addition to squashing the above patch into
your patch #6, you would need to move the creation of the trailer.h header
file from patch 07/11 ("trailer: add interpret-trailers command") to patch
06/11 ("trailer: put all the processing together and print"), where it should
have been anyway! :-D
HTH
ATB,
Ramsay Jones
> Christian Couder (11):
> Add data structures and basic functions for commit trailers
> trailer: process trailers from stdin and arguments
> trailer: read and process config information
> trailer: process command line trailer arguments
> trailer: parse trailers from stdin
> trailer: put all the processing together and print
> trailer: add interpret-trailers command
> trailer: add tests for "git interpret-trailers"
> trailer: execute command from 'trailer..command'
> trailer: add tests for commands in config file
> Documentation: add documentation for 'git interpret-trailers'
>
> .gitignore | 1 +
> Documentation/git-interpret-trailers.txt | 123 ++
> Makefile | 2 +
> builtin.h| 1 +
> builtin/interpret-trailers.c | 33 ++
> git.c| 1 +
> t/t7513-interpret-trailers.sh| 261
> trailer.c| 661
> +++
> trailer.h| 6 +
> 9 files changed, 1089 insertions(+)
> create mode 100644 Documentation/git-interpret-trailers.txt
> create mode 100644 builtin/interpret-trailers.c
> create mode 100755 t/t7513-interpret-trailers.sh
> create mode 100644 trailer.c
> create mode 100644 trailer.h
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Makefile: Fix compilation of windows resource file
On 21/01/14 21:36, Junio C Hamano wrote:
> Junio C Hamano writes:
>
>> Ramsay Jones writes:
>>
>>> If the git version number consists of less than three period
>>> separated numbers, then the windows resource file compilation
>>> issues a syntax error:
>>>
>>> $ touch git.rc
>>> $ make V=1 git.res
>>> GIT_VERSION = 1.9.rc0
>>> windres -O coff \
>>> -DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \
>>> -DGIT_VERSION="\\\"1.9.rc0\\\"" git.rc -o git.res
>>> C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error
>>> make: *** [git.res] Error 1
>>> $
>>>
>>> [Note that -DPATCH=rc0]
>>
>> Thanks for a report. I've been wondering how many distros and
>> packagers would have an issue like this when we go to 2-digit
>> release naming. Of course we knew everybody can grok 3-or-4 ;-)
>>
>>> In order to fix the syntax error, we replace any rcX with zero and
>>> include some additional 'zero' padding to the version number list.
>>>
>>> Signed-off-by: Ramsay Jones
>>> ---
>>>
>>> Hi Junio,
>>>
>>> This patch is marked RFC because, as I was just about to send this
>>> email, I realized it wouldn't always work:
>>
>> Yeah, and I suspect that with the use of $(wordlist 1,3,...) it is
>> not even working for maintenance releases. Does it differenciate
>> between 1.8.5.1 and 1.8.5.2, for example?. Or does "windres" always
>> assume that a package version is always 3-dewey-decimal (not 2, not
>> 4)?
I'm no expert on '.rc' file syntax, but the code certainly does not
(currently) support four digit versions.
> Perhaps like this? Just grab digit-only segments that are separated
> with either dot or dash (and stop when we see a non-digit like
> 'dirty' or 'rcX'), and make them separated with comma.
Oh, this is *much* better than my new (unsent) attempt to fix this! ;-)
>
> Note that I am merely guessing that "short-digit" version numbers
> are acceptable by now after seeing
>
> https://sourceware.org/ml/binutils/2012-07/msg00199.html
Ah, nice find!
I will test your patch (below) and let you know soon, but it looks
good to me. (I can't test it tonight, unfortunately.)
ATB,
Ramsay Jones
>
> without knowing the current state of affairs. If that is not the
> case you may have to count the iteration of the loop and append or
> chop the resulting string as necessary.
>
> Makefile | 2 +-
> gen-version-string.sh | 13 +
> git.rc| 4 ++--
> 3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b4af1e2..329f942 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
>
> git.res: git.rc GIT-VERSION-FILE
> $(QUIET_RC)$(RC) \
> - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst
> ., ,$(GIT_VERSION) \
> + -DVERSIONSTRING=$$(./gen-version-string.sh $(GIT_VERSION)) \
> -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@
>
> ifndef NO_PERL
> diff --git a/gen-version-string.sh b/gen-version-string.sh
> new file mode 100755
> index 000..00af718
> --- /dev/null
> +++ b/gen-version-string.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +IFS=.- result=
> +for v in $1
> +do
> + if expr "$v" : '[0-9][0-9]*$' >/dev/null
> + then
> + result=$result${result:+,}$v
> + else
> + break
> + fi
> +done
> +echo "$result"
> diff --git a/git.rc b/git.rc
> index bce6db9..6f2a8d2 100644
> --- a/git.rc
> +++ b/git.rc
> @@ -1,6 +1,6 @@
> 1 VERSIONINFO
> -FILEVERSION MAJOR,MINOR,PATCH,0
> -PRODUCTVERSION MAJOR,MINOR,PATCH,0
> +FILEVERSION VERSIONSTRING,0
> +PRODUCTVERSION VERSIONSTRING,0
> BEGIN
>BLOCK "StringFileInfo"
>BEGIN
> .
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] Makefile: Fix compilation of windows resource file
If the git version number consists of less than three period separated numbers, then the windows resource file compilation issues a syntax error: $ touch git.rc $ make V=1 git.res GIT_VERSION = 1.9.rc0 windres -O coff \ -DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \ -DGIT_VERSION="\\\"1.9.rc0\\\"" git.rc -o git.res C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error make: *** [git.res] Error 1 $ [Note that -DPATCH=rc0] In order to fix the syntax error, we replace any rcX with zero and include some additional 'zero' padding to the version number list. Signed-off-by: Ramsay Jones --- Hi Junio, This patch is marked RFC because, as I was just about to send this email, I realized it wouldn't always work: $ touch git.rc $ make V=1 GIT_VERSION=1.9.dirty git.res windres -O coff \ -DMAJOR=1 -DMINOR=9 -DPATCH=dirty \ -DGIT_VERSION="\\\"1.9.dirty\\\"" git.rc -o git.res C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error make: *** [git.res] Error 1 $ :-D I suspect it would be easier to change GIT-VERSION-GEN to also set, say, GIT_VERSION_MAJOR, GIT_VERSION_MINOR and GIT_VERSION_PATCH ... ATB, Ramsay Jones Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b4af1e2..308baaa 100644 --- a/Makefile +++ b/Makefile @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES git.res: git.rc GIT-VERSION-FILE $(QUIET_RC)$(RC) \ - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, ,$(subst ., ,$(GIT_VERSION) \ + $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(patsubst rc%,0,$(subst -, ,$(subst ., ,$(GIT_VERSION))) 0 0))) \ -DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" $< -o $@ ifndef NO_PERL -- 1.8.5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] .gitignore: Ignore editor backup and swap files
On 16/01/14 22:06, Junio C Hamano wrote: > Alexander Berntsen writes: > >> Signed-off-by: Alexander Berntsen >> --- >> .gitignore | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/.gitignore b/.gitignore >> index b5f9def..2905c21 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -240,3 +240,5 @@ >> *.pdb >> /Debug/ >> /Release/ >> +*~ >> +.*.swp > > I personally do not mind listing these common ones too much, but if > I am not mistaken, our policy on this file so far has been that it > lists build artifacts, and not personal preference (the *.swp entry > is useless for those who never use vim, for example). > > These paths that depend on your choice of the editor and other tools > can still be managed in your personal .git/info/exclude in the > meantime. As a vim user, I have these set in my ~/.gitignore file, which I refer to from ~/.gitconfig using core.excludesfile. ;-) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] shallow: Remove unused code
Commit 58babfff ("shallow.c: the 8 steps to select new commits for
.git/shallow", 05-12-2013) added a function to implement step 5 of
the quoted eight steps, namely 'remove_nonexistent_ours_in_pack()'.
This function implements an optional optimization step in the new
shallow commit selection algorithm. However, this function has no
callers. (The commented out call sites would need to change, in
order to provide information required by the function.)
Signed-off-by: Ramsay Jones
---
Hi Junio,
This should, perhaps, be marked as RFC; if the patch to actually
use this function is coming soon, then this is not needed.
However, I suspect it could be slow in coming ... :-P
ATB,
Ramsay Jones
builtin/receive-pack.c | 1 -
commit.h | 2 --
fetch-pack.c | 1 -
shallow.c | 16
4 files changed, 20 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index de869b5..85bba35 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1059,7 +1059,6 @@ static void update_shallow_info(struct command *commands,
struct command *cmd;
int *ref_status;
remove_nonexistent_theirs_shallow(si);
- /* XXX remove_nonexistent_ours_in_pack() */
if (!si->nr_ours && !si->nr_theirs) {
shallow_update = 0;
return;
diff --git a/commit.h b/commit.h
index 5507460..30598c6 100644
--- a/commit.h
+++ b/commit.h
@@ -230,8 +230,6 @@ struct shallow_info {
extern void prepare_shallow_info(struct shallow_info *, struct sha1_array *);
extern void clear_shallow_info(struct shallow_info *);
extern void remove_nonexistent_theirs_shallow(struct shallow_info *);
-extern void remove_nonexistent_ours_in_pack(struct shallow_info *,
- struct packed_git *);
extern void assign_shallow_commits_to_refs(struct shallow_info *info,
uint32_t **used,
int *ref_status);
diff --git a/fetch-pack.c b/fetch-pack.c
index 1d0328d..d52de74 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -984,7 +984,6 @@ static void update_shallow(struct fetch_pack_args *args,
return;
remove_nonexistent_theirs_shallow(si);
- /* XXX remove_nonexistent_ours_in_pack() */
if (!si->nr_ours && !si->nr_theirs)
return;
for (i = 0; i < nr_sought; i++)
diff --git a/shallow.c b/shallow.c
index f48f732..bbc98b5 100644
--- a/shallow.c
+++ b/shallow.c
@@ -358,22 +358,6 @@ void remove_nonexistent_theirs_shallow(struct shallow_info
*info)
info->nr_theirs = dst;
}
-/* Step 5, remove non-existent ones in "ours" in the pack */
-void remove_nonexistent_ours_in_pack(struct shallow_info *info,
-struct packed_git *p)
-{
- unsigned char (*sha1)[20] = info->shallow->sha1;
- int i, dst;
- trace_printf_key(TRACE_KEY, "shallow:
remove_nonexistent_ours_in_pack\n");
- for (i = dst = 0; i < info->nr_ours; i++) {
- if (i != dst)
- info->ours[dst] = info->ours[i];
- if (find_pack_entry_one(sha1[info->ours[i]], p))
- dst++;
- }
- info->nr_ours = dst;
-}
-
define_commit_slab(ref_bitmap, uint32_t *);
struct paint_info {
--
1.8.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] send-pack.c: mark a file-local function static
Commit f2c681cf ("send-pack: support pushing from a shallow clone
via http", 05-12-2013) adds the 'advertise_shallow_grafts_buf'
function as an external symbol.
Noticed by sparse. ("'advertise_shallow_grafts_buf' was not declared.
Should it be static?")
Signed-off-by: Ramsay Jones
---
send-pack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/send-pack.c b/send-pack.c
index 2a199cc..6129b0f 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -183,7 +183,7 @@ static int advertise_shallow_grafts_cb(const struct
commit_graft *graft, void *c
return 0;
}
-void advertise_shallow_grafts_buf(struct strbuf *sb)
+static void advertise_shallow_grafts_buf(struct strbuf *sb)
{
if (!is_repository_shallow())
return;
--
1.8.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 23/23] compat/mingw.h: Fix the MinGW and msvc builds
On 28/12/13 10:00, Jeff King wrote:
> On Wed, Dec 25, 2013 at 11:08:57PM +0100, Erik Faye-Lund wrote:
>
>> On Sat, Dec 21, 2013 at 3:00 PM, Jeff King wrote:
>>> From: Ramsay Jones
>>>
>>> Signed-off-by: Ramsay Jones
>>> Signed-off-by: Junio C Hamano
>>> Signed-off-by: Jeff King
>>> ---
>>> compat/mingw.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/compat/mingw.h b/compat/mingw.h
>>> index 92cd728..8828ede 100644
>>> --- a/compat/mingw.h
>>> +++ b/compat/mingw.h
>>> @@ -345,6 +345,7 @@ static inline char *mingw_find_last_dir_sep(const char
>>> *path)
>>> #define PATH_SEP ';'
>>> #define PRIuMAX "I64u"
>>> #define PRId64 "I64d"
>>> +#define PRIx64 "I64x"
>>>
>>
>> Please, move this before patch #8, and adjust the commit message.
>
> Yeah, that makes sense. Though I think we can do one better and simply
> remove the need for it entirely. The only use of PRIx64 is in a
> debugging function that does not get called.
>
> How about squashing the patch below into patch 8 ("ewah: compressed
> bitmap implementation"):
>
> diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
> index f104b87..9ced2da 100644
> --- a/ewah/ewah_bitmap.c
> +++ b/ewah/ewah_bitmap.c
> @@ -381,18 +381,6 @@ void ewah_iterator_init(struct ewah_iterator *it, struct
> ewah_bitmap *parent)
> read_new_rlw(it);
> }
>
> -void ewah_dump(struct ewah_bitmap *self)
> -{
> - size_t i;
> - fprintf(stderr, "%"PRIuMAX" bits | %"PRIuMAX" words | ",
> - (uintmax_t)self->bit_size, (uintmax_t)self->buffer_size);
> -
> - for (i = 0; i < self->buffer_size; ++i)
> - fprintf(stderr, "%016"PRIx64" ", (uint64_t)self->buffer[i]);
> -
> - fprintf(stderr, "\n");
> -}
> -
> void ewah_not(struct ewah_bitmap *self)
> {
> size_t pointer = 0;
> diff --git a/ewah/ewok.h b/ewah/ewok.h
> index 619afaa..43adeb5 100644
> --- a/ewah/ewok.h
> +++ b/ewah/ewok.h
> @@ -193,8 +193,6 @@ void ewah_and(
> struct ewah_bitmap *ewah_j,
> struct ewah_bitmap *out);
>
> -void ewah_dump(struct ewah_bitmap *self);
> -
> /**
> * Direct word access
> */
I'm always in favour of removing unused (or unwanted) code! :-D
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] safe_create_leading_directories(): fix a mkdir/rmdir race
On 22/12/13 07:14, Michael Haggerty wrote:
> It could be that some other process is trying to clean up empty
> directories at the same time that safe_create_leading_directories() is
> attempting to create them. In this case, it could happen that
> directory "a/b" was present at the end of one iteration of the loop
> (either it was already present or we just created it ourselves), but
> by the time we try to create directory "a/b/c", directory "a/b" has
> been deleted. In fact, directory "a" might also have been deleted.
>
> So, if a call to mkdir() fails with ENOENT, then try checking/making
> all directories again from the beginning. Attempt up to three times
> before giving up.
>
> Signed-off-by: Michael Haggerty
> ---
> sha1_file.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index dcfd35a..abcb56b 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -108,6 +108,7 @@ int mkdir_in_gitdir(const char *path)
> int safe_create_leading_directories(char *path)
> {
> char *next_component = path + offset_1st_component(path);
> + int attempts = 3;
> int retval = 0;
>
> while (!retval && next_component) {
> @@ -132,6 +133,16 @@ int safe_create_leading_directories(char *path)
> if (errno == EEXIST &&
> !stat(path, &st) && S_ISDIR(st.st_mode)) {
> ; /* somebody created it since we checked */
> + } else if (errno == ENOENT && --attempts) {
> + /*
> + * Either mkdir() failed bacause
s/bacause/because/
> + * somebody just pruned the containing
> + * directory, or stat() failed because
> + * the file that was in our way was
> + * just removed. Either way, try
> + * again from the beginning:
> + */
> + next_component = path +
> offset_1st_component(path);
> } else {
> retval = -1;
> }
>
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/21] path.c: rename vsnpath() to git_vsnpath()
On 16/12/13 17:11, Jonathan Nieder wrote:
> Duy Nguyen wrote:
>>>> Ramsay Jones wrote:
>
>>>>> :-D I renamed this _from_ git_vsnpath() in commit 5b3b8fa2 ("path.c:
>>>>> Remove the
>>>>> 'git_' prefix from a file scope function", 04-09-2012), because ... well
>>>>> it's a
>>>>> file scope function! (i.e. the git_ prefix implies greater than file
>>>>> scope).
>>>>> I'm not very good at naming things, so ...
> [...]
>> OK I go with this. I think it makes sense
>>
>> vsnpath -> do_git_path
>
> I think this renaming would be still losing clarity --- it loses the
> information that this is the vsnprintf-like variant of git_path.
>
> Do we actually have a convention that functions with git_ prefix
> should be global?
Hmm, probably not no. However, any symbol that starts with git_ positively
screams global symbol to me. Maybe, I just need to close my ears. ;-)
I didn't intend to provoke so much discussion. I was simply pointing out
that this symbol had already been renamed, in the exact opposite direction,
once before. Please just ignore me.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/21] path.c: rename vsnpath() to git_vsnpath()
On 15/12/13 02:25, Duy Nguyen wrote:
> On Sun, Dec 15, 2013 at 3:23 AM, Ramsay Jones
> wrote:
>> On 14/12/13 10:54, Nguyễn Thái Ngọc Duy wrote:
>>> This is the underlying implementation of git_path(), git_pathdup() and
>>> git_snpath() which will prefix $GIT_DIR in the result string. Put git_
>>> prefix in front of it to avoid the confusion that this is a generic
>>> path handling function.#
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy
>>> ---
>>> path.c | 8
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/path.c b/path.c
>>> index 4c1c144..06863b7 100644
>>> --- a/path.c
>>> +++ b/path.c
>>> @@ -50,7 +50,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
>>> return cleanup_path(buf);
>>> }
>>>
>>> -static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args)
>>> +static char *git_vsnpath(char *buf, size_t n, const char *fmt, va_list
>>> args)
>>
>> :-D I renamed this _from_ git_vsnpath() in commit 5b3b8fa2 ("path.c: Remove
>> the
>> 'git_' prefix from a file scope function", 04-09-2012), because ... well
>> it's a
>> file scope function! (i.e. the git_ prefix implies greater than file scope).
>> I'm not very good at naming things, so ...
>
> maybe gitdir_vsnpath() then to avoid the global scope prefix git_?
Sounds fine to me (but then so does vsnpath ;-) ).
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/21] path.c: rename vsnpath() to git_vsnpath()
On 14/12/13 10:54, Nguyễn Thái Ngọc Duy wrote:
> This is the underlying implementation of git_path(), git_pathdup() and
> git_snpath() which will prefix $GIT_DIR in the result string. Put git_
> prefix in front of it to avoid the confusion that this is a generic
> path handling function.#
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> path.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/path.c b/path.c
> index 4c1c144..06863b7 100644
> --- a/path.c
> +++ b/path.c
> @@ -50,7 +50,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
> return cleanup_path(buf);
> }
>
> -static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args)
> +static char *git_vsnpath(char *buf, size_t n, const char *fmt, va_list args)
:-D I renamed this _from_ git_vsnpath() in commit 5b3b8fa2 ("path.c: Remove the
'git_' prefix from a file scope function", 04-09-2012), because ... well it's a
file scope function! (i.e. the git_ prefix implies greater than file scope).
I'm not very good at naming things, so ...
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] send-pack.c: mark a file-local function static
On 13/12/13 00:58, Duy Nguyen wrote: > On Fri, Dec 13, 2013 at 6:15 AM, Ramsay Jones > wrote: >> BTW, I have not been following these patches, but I noticed that the >> 'remove_nonexistent_ours_in_pack()' function has no callers. (There are >> two commented out callers - but they seem to have *always* been commented >> out ;-) ). So, step 5 is no longer required? :-D > > It's more of an optimization than a requirement, to avoid expensive > calculation later. Uncommenting is possible but I need to pass the > pack file name around a bit. > Ah, OK. (I should probably refrain from commenting on code I haven't read ... :-P ) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] send-pack.c: mark a file-local function static
Commit f2c681cf ("send-pack: support pushing from a shallow clone
via http", 05-12-2013) adds the 'advertise_shallow_grafts_buf'
function as an external symbol. This symbol does not require
more than file visibility.
Noticed by sparse. ("'advertise_shallow_grafts_buf' was not declared.
Should it be static?")
Signed-off-by: Ramsay Jones
---
Hi Duy,
If you need to re-roll the patches in your 'nd/shallow-clone' branch,
could you please squash this into your patch. Thanks!
BTW, I have not been following these patches, but I noticed that the
'remove_nonexistent_ours_in_pack()' function has no callers. (There are
two commented out callers - but they seem to have *always* been commented
out ;-) ). So, step 5 is no longer required? :-D
ATB,
Ramsay Jones
send-pack.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/send-pack.c b/send-pack.c
index 2a199cc..6129b0f 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -183,7 +183,7 @@ static int advertise_shallow_grafts_cb(const struct
commit_graft *graft, void *c
return 0;
}
-void advertise_shallow_grafts_buf(struct strbuf *sb)
+static void advertise_shallow_grafts_buf(struct strbuf *sb)
{
if (!is_repository_shallow())
return;
--
1.8.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')
On 20/11/13 17:22, Junio C Hamano wrote:
> Ramsay Jones writes:
>
>> Signed-off-by: Ramsay Jones
>> ---
>>
>> Hi Jens,
>>
>> commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
>> from the commit message", 19-11-2013) in 'pu' fails the new test
>> it added to t7507.
>>
>> I didn't spend too long looking at the problem, so take this patch
>> as nothing more than a quick suggestion for a possible solution! :-P
>> [The err file contained something like: "There was a problem with the
>> editor '"$FAKE_EDITOR"'"].
>>
>> Having said that, this fixes it for me ...
>
> Well spotted. test_must_fail being a shell function, not a command,
> we shouldn't have used the "VAR=val cmd" one-shot environment
> assignment for portability.
>
> Even though this happens to be the last test in the script, using
> test_set_editor to permanently affect the choice of editor for tests
> that follow is not generally a good idea. It would be safer to do
> this, I would have to say:
>
> git commit -a -m "submodule commit"
> ) &&
> (
> GIT_EDITOR=cat &&
> export GIT_EDITOR &&
> test_must_fail git commit -a -v 2>err
> ) &&
> test_i18ngrep "Aborting ..."
Yes, this works great (and I very nearly wrote exactly this, but since
the test was using test_set_editor anyway ...) :-D
Thanks.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t7507-*.sh: Fix test #8 (could not run '"$FAKE_EDITOR"')
Signed-off-by: Ramsay Jones
---
Hi Jens,
commit 61b6a633 ("commit -v: strip diffs and submodule shortlogs
from the commit message", 19-11-2013) in 'pu' fails the new test
it added to t7507.
I didn't spend too long looking at the problem, so take this patch
as nothing more than a quick suggestion for a possible solution! :-P
[The err file contained something like: "There was a problem with the
editor '"$FAKE_EDITOR"'"].
Having said that, this fixes it for me ...
ATB,
Ramsay Jones
t/t7507-commit-verbose.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index 09c1150..49cfb3c 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -79,7 +79,8 @@ test_expect_success 'submodule log is stripped out too with
-v' '
echo "more" >>file &&
git commit -a -m "submodule commit"
) &&
- GIT_EDITOR=cat test_must_fail git commit -a -v 2>err &&
+ test_set_editor cat &&
+ test_must_fail git commit -a -v 2>err &&
test_i18ngrep "Aborting commit due to empty commit message." err
'
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/21] pack bitmaps
On 14/11/13 23:09, Ramsay Jones wrote: > On 14/11/13 21:33, Jeff King wrote: >> On Thu, Nov 14, 2013 at 07:19:38PM +0000, Ramsay Jones wrote: >> >>> Unfortunately, I didn't find time this weekend to finish the msvc build >>> fixes. However, after a quick squint at these patches, I think you have >>> almost done it for me! :-D >>> >>> I must have misunderstood the previous discussion, because my patch was >>> written on the assumption that the ewah directory wouldn't be "git-ified" >>> (e.g. #include git-compat-util.h). >> >> I think it was up for debate at some point, but we did decide to go >> ahead and git-ify. Please feel free to submit further fixups if you need >> them. > > Yep, will do; at present it looks like that one-liner. Despite saying I would wait for these patches to land in pu, I applied these patches to the next branch (@ 8721652 "Sync with 1.8.5-rc2") so that I could try them out. As expected, everything was fine on Linux and cygwin, and the msvc build needed a one-liner fix. However, I didn't expect that MinGW would need the same fix! ;-) I had forgotten that "git-compat-util.h" does not include the header on MinGW (my previous patch included that header directly), so that we have to add the printf format macros directly to the compat/mingw.h header. [I assume that an earlier version of the library on MinGW did not have the and headers. We could now include that header on MinGW and move the PRIuMAX, PRId64 and PRIx64 macros to compat/msvc.h, but I didn't think it was worth the churn ... ] The one-liner is given below ... ATB, Ramsay Jones -- >8 -- Subject: [PATCH] compat/mingw.h: Fix the MinGW and msvc builds Signed-off-by: Ramsay Jones --- compat/mingw.h | 1 + 1 file changed, 1 insertion(+) diff --git a/compat/mingw.h b/compat/mingw.h index 92cd728..8828ede 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -345,6 +345,7 @@ static inline char *mingw_find_last_dir_sep(const char *path) #define PATH_SEP ';' #define PRIuMAX "I64u" #define PRId64 "I64d" +#define PRIx64 "I64x" void mingw_open_html(const char *path); #define open_html mingw_open_html -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/21] pack bitmaps
On 14/11/13 21:33, Jeff King wrote: > On Thu, Nov 14, 2013 at 07:19:38PM +0000, Ramsay Jones wrote: > >> Unfortunately, I didn't find time this weekend to finish the msvc build >> fixes. However, after a quick squint at these patches, I think you have >> almost done it for me! :-D >> >> I must have misunderstood the previous discussion, because my patch was >> written on the assumption that the ewah directory wouldn't be "git-ified" >> (e.g. #include git-compat-util.h). > > I think it was up for debate at some point, but we did decide to go > ahead and git-ify. Please feel free to submit further fixups if you need > them. Yep, will do; at present it looks like that one-liner. > >>> - the ewah code used gcc's __builtin_ctzll, but did not provide a >>> suitable fallback. We now provide a fallback in C. >> >> ... here. >> >> I was messing around with several implementations (including the use of >> msvc compiler intrinsics) with the intention of doing some timing tests >> etc. [I suspected my C fallback function (a different implementation to >> yours) would be slightly faster.] > > Yeah, I looked around for several implementations, and ultimately wrote > one that was the most readable to me. The one I found shortest and most > inscrutable was: > > return popcount((x & -x) - 1); > > the details of which I still haven't worked through in my head. ;) Yeah, I stumbled over that one too! > I do think on most platforms that intrinsics or inline assembler are the > way to go. My main goal was to get something correct that would let it > compile everywhere, and then people can use that as a base for > optimizing. Patches welcome. :) Indeed, I can happily leave that to another day (or to someone else more motivated ;-) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/21] pack bitmaps
On 14/11/13 12:41, Jeff King wrote: > Here's another iteration of the pack bitmaps series. Compared to v2, it > changes: > > - misc style/typo fixes > > - portability fixes from Ramsay and Torsten Unfortunately, I didn't find time this weekend to finish the msvc build fixes. However, after a quick squint at these patches, I think you have almost done it for me! :-D I must have misunderstood the previous discussion, because my patch was written on the assumption that the ewah directory wouldn't be "git-ified" (e.g. #include git-compat-util.h). So, most of my patch is no longer necessary, given the use of the git compat header (and removal of system headers). I suspect that you only need to add an '#define PRIx64 "I64x"' definition (Hmm, probably to the compat/mingw.h header). I won't know for sure until I actually try them out, of course. I will wait until these patches land in pu. [Note: the msvc build is still broken, but the failure is not caused by these patches. Unfortunately, the tests in t5310-*.sh fail. However, if I include some debug code, the tests pass ... :-P ] The part of the patch I was still working on was ... > > - count-objects garbage-reporting patch from Duy > > - disable bitmaps when is_repository_shallow(); this also covers the >case where the client is shallow, since we feed pack-objects a >--shallow-file in that case. This used to done by checking >!internal_rev_list, but that doesn't apply after cdab485. > > - ewah sources now properly use git-compat-util.h and do not include >system headers > > - the ewah code uses ewah_malloc, ewah_realloc, and so forth to let the >project use a particular allocator (and we want to use xmalloc and >friends). And we defined those in pack-bitmap.h, but of course that >had no effect on the ewah/*.c files that did not include >pack-bitmap.h. Since we are hacking up and git-ifying libewok >anyway, we can just set the hardcoded fallback to xmalloc instead of >malloc. > > - the ewah code used gcc's __builtin_ctzll, but did not provide a > suitable fallback. We now provide a fallback in C. ... here. I was messing around with several implementations (including the use of msvc compiler intrinsics) with the intention of doing some timing tests etc. [I suspected my C fallback function (a different implementation to yours) would be slightly faster.] ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] fix up 'jk/pack-bitmap' branch
On 08/11/13 17:10, Torsten Bögershausen wrote: > On 2013-11-07 23.19, Jeff King wrote: >> On Thu, Nov 07, 2013 at 09:58:02PM +0000, Ramsay Jones wrote: >> >>> These patches fix various errors/warnings on the cygwin, MinGW and >>> msvc builds, provoked by the jk/pack-bitmap branch. >> >> Thanks. Your timing is impeccable, as I was just sitting down to >> finalize the next re-roll. I'll add these in. >> >> -Peff > > Side question: > Do we have enough test coverage for htonll()/ntohll(), > or do we want do the "module test" which I send a couple of days before ? Yes, sorry for not bringing that up; I didn't get to try (or even look at) your test patch, because I couldn't 'git am' it. :( I've been meaning to look into why that is, but just haven't had time yet. (I think it may have something to do with your name - I've noticed in the past that any mail with utf8 characters causes problems.) However, I should have alerted Jeff to your patch; sorry about that! ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] Makefile: Add object files in ewah/ to clean target
Signed-off-by: Ramsay Jones --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 07b0626..1950858 100644 --- a/Makefile +++ b/Makefile @@ -2484,8 +2484,9 @@ profile-clean: $(RM) $(addsuffix *.gcno,$(addprefix $(PROFILE_DIR)/, $(object_dirs))) clean: profile-clean coverage-clean - $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o vcs-svn/*.o \ - builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) + $(RM) *.o *.res block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o + $(RM) xdiff/*.o vcs-svn/*.o ewah/*.o builtin/*.o + $(RM) $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X $(RM) $(TEST_PROGRAMS) $(NO_INSTALL) $(RM) -r bin-wrappers $(dep_dirs) -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] pack-objects: Limit visibility of 'indexed_commits' symbols
Noticed by sparse. ("symbol '...' was not declared. Should it be
static?")
Signed-off-by: Ramsay Jones
---
builtin/pack-objects.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 423e85a..161bfc2 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -85,9 +85,9 @@ static uint32_t reused, reused_delta;
/*
* Indexed commits
*/
-struct commit **indexed_commits;
-unsigned int indexed_commits_nr;
-unsigned int indexed_commits_alloc;
+static struct commit **indexed_commits;
+static unsigned int indexed_commits_nr;
+static unsigned int indexed_commits_alloc;
static void index_commit_for_bitmap(struct commit *commit)
{
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] khash.h: Spell the null pointer as NULL
Noticed by sparse. ("Using plain integer as NULL pointer")
Signed-off-by: Ramsay Jones
---
khash.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/khash.h b/khash.h
index 0fdf39d..c4c1613 100644
--- a/khash.h
+++ b/khash.h
@@ -114,7 +114,7 @@ static const double __ac_HASH_UPPER = 0.77;
}
\
SCOPE int kh_resize_##name(kh_##name##_t *h, khint_t new_n_buckets) \
{ /* This function uses 0.25*n_buckets bytes of working space instead
of [sizeof(key_t+val_t)+.25]*n_buckets. */ \
- khint32_t *new_flags = 0;
\
+ khint32_t *new_flags = NULL;
\
khint_t j = 1;
\
{
\
kroundup32(new_n_buckets);
\
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] ewah_bitmap.c: Fix printf format warnings on MinGW
On MinGW, gcc complains as follows:
CC ewah/ewah_bitmap.o
ewah/ewah_bitmap.c: In function 'ewah_dump':
ewah/ewah_bitmap.c:389: warning: unknown conversion type \
character 'z' in format
ewah/ewah_bitmap.c:389: warning: unknown conversion type \
character 'z' in format
ewah/ewah_bitmap.c:389: warning: too many arguments for format
ewah/ewah_bitmap.c:392: warning: unknown conversion type \
character 'l' in format
ewah/ewah_bitmap.c:392: warning: too many arguments for format
In order to suppress the warnings, use the PRIuMAX and PRIx64 macros
from the header file.
Signed-off-by: Ramsay Jones
---
ewah/ewah_bitmap.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c
index 625f5a6..1e363b9 100644
--- a/ewah/ewah_bitmap.c
+++ b/ewah/ewah_bitmap.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include "ewok.h"
#include "ewok_rlw.h"
@@ -386,10 +387,11 @@ void ewah_iterator_init(struct ewah_iterator *it, struct
ewah_bitmap *parent)
void ewah_dump(struct ewah_bitmap *self)
{
size_t i;
- fprintf(stderr, "%zu bits | %zu words | ", self->bit_size,
self->buffer_size);
+ fprintf(stderr, "%"PRIuMAX" bits | %"PRIuMAX" words | ",
+ (uintmax_t)self->bit_size, (uintmax_t)self->buffer_size);
for (i = 0; i < self->buffer_size; ++i)
- fprintf(stderr, "%016llx ", (unsigned long
long)self->buffer[i]);
+ fprintf(stderr, "%016"PRIx64" ", (unsigned long
long)self->buffer[i]);
fprintf(stderr, "\n");
}
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] fix up 'jk/pack-bitmap' branch
Hi Jeff, These patches fix various errors/warnings on the cygwin, MinGW and msvc builds, provoked by the jk/pack-bitmap branch. Note that this does not fix all problems on the msvc build; I have a solution, but I don't like it. :-D So, I'm going to try a different fix. I had hoped to have done this by now, but ... (hopefully, sometime this weekend). Note that I have only tested the cygwin and MinGW builds by running the t5310-pack-bitmaps.sh test, and only on a little-endian machine. (Torsten has tested the first patch on a big-endian) Note also, that these patches are build on top of the 'pu' branch as of yesterday (pu @ 2b65d9ebc). So, could you please squash these patches into the relevant commits on your branch. Thanks! ATB, Ramsay Jones Ramsay Jones (5): compat/bswap.h: Fix build on cygwin, MinGW and msvc Makefile: Add object files in ewah/ to clean target khash.h: Spell the null pointer as NULL pack-objects: Limit visibility of 'indexed_commits' symbols ewah_bitmap.c: Fix printf format warnings on MinGW Makefile | 5 +-- builtin/pack-objects.c | 6 ++-- compat/bswap.h | 97 +++--- ewah/ewah_bitmap.c | 6 ++-- khash.h| 2 +- 5 files changed, 79 insertions(+), 37 deletions(-) -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] compat/bswap.h: Fix build on cygwin, MinGW and msvc
Signed-off-by: Ramsay Jones
---
compat/bswap.h | 97 --
1 file changed, 68 insertions(+), 29 deletions(-)
diff --git a/compat/bswap.h b/compat/bswap.h
index ea1a9ed..c18a78e 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -17,7 +17,20 @@ static inline uint32_t default_swab32(uint32_t val)
((val & 0x00ff) << 24));
}
+static inline uint64_t default_bswap64(uint64_t val)
+{
+ return (((val & (uint64_t)0x00ffULL) << 56) |
+ ((val & (uint64_t)0xff00ULL) << 40) |
+ ((val & (uint64_t)0x00ffULL) << 24) |
+ ((val & (uint64_t)0xff00ULL) << 8) |
+ ((val & (uint64_t)0x00ffULL) >> 8) |
+ ((val & (uint64_t)0xff00ULL) >> 24) |
+ ((val & (uint64_t)0x00ffULL) >> 40) |
+ ((val & (uint64_t)0xff00ULL) >> 56));
+}
+
#undef bswap32
+#undef bswap64
#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
@@ -32,54 +45,80 @@ static inline uint32_t git_bswap32(uint32_t x)
return result;
}
+#define bswap64 git_bswap64
+#if defined(__x86_64__)
+static inline uint64_t git_bswap64(uint64_t x)
+{
+ uint64_t result;
+ if (__builtin_constant_p(x))
+ result = default_bswap64(x);
+ else
+ __asm__("bswap %q0" : "=r" (result) : "0" (x));
+ return result;
+}
+#else
+static inline uint64_t git_bswap64(uint64_t x)
+{
+ union { uint64_t i64; uint32_t i32[2]; } tmp, result;
+ if (__builtin_constant_p(x))
+ result.i64 = default_bswap64(x);
+ else {
+ tmp.i64 = x;
+ result.i32[0] = git_bswap32(tmp.i32[1]);
+ result.i32[1] = git_bswap32(tmp.i32[0]);
+ }
+ return result.i64;
+}
+#endif
+
#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
#include
#define bswap32(x) _byteswap_ulong(x)
+#define bswap64(x) _byteswap_uint64(x)
#endif
-#ifdef bswap32
+#if defined(bswap32)
#undef ntohl
#undef htonl
#define ntohl(x) bswap32(x)
#define htonl(x) bswap32(x)
-#ifndef __BYTE_ORDER
-# if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN)
-# define __BYTE_ORDER BYTE_ORDER
-# define __LITTLE_ENDIAN LITTLE_ENDIAN
-# define __BIG_ENDIAN BIG_ENDIAN
-# else
-# error "Cannot determine endianness"
-# endif
+#endif
+
+#if defined(bswap64)
+
+#undef ntohll
+#undef htonll
+#define ntohll(x) bswap64(x)
+#define htonll(x) bswap64(x)
+
+#else
+
+#undef ntohll
+#undef htonll
+
+#if !defined(__BYTE_ORDER)
+# if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN)
+# define __BYTE_ORDER BYTE_ORDER
+# define __LITTLE_ENDIAN LITTLE_ENDIAN
+# define __BIG_ENDIAN BIG_ENDIAN
+# endif
+#endif
+
+#if !defined(__BYTE_ORDER)
+# error "Cannot determine endianness"
#endif
#if __BYTE_ORDER == __BIG_ENDIAN
# define ntohll(n) (n)
# define htonll(n) (n)
-#elif __BYTE_ORDER == __LITTLE_ENDIAN
-# if defined(__GNUC__) && defined(__GLIBC__)
-# include
-# else /* GNUC & GLIBC */
-static inline uint64_t bswap_64(uint64_t val)
-{
- return ((val & (uint64_t)0x00ffULL) << 56)
- | ((val & (uint64_t)0xff00ULL) << 40)
- | ((val & (uint64_t)0x00ffULL) << 24)
- | ((val & (uint64_t)0xff00ULL) << 8)
- | ((val & (uint64_t)0x00ffULL) >> 8)
- | ((val & (uint64_t)0xff00ULL) >> 24)
- | ((val & (uint64_t)0x00ffULL) >> 40)
- | ((val & (uint64_t)0xff00ULL) >> 56);
-}
-# endif /* GNUC & GLIBC */
-# define ntohll(n) bswap_64(n)
-# define htonll(n) bswap_64(n)
-#else /* __BYTE_ORDER */
-# error "Can't define htonll or ntohll!"
+#else
+# define ntohll(n) default_bswap64(n)
+# define htonll(n) default_bswap64(n)
#endif
#endif
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: htonll, ntohll
On 31/10/13 13:24, Torsten Bögershausen wrote:
> On 2013-10-30 22.07, Ramsay Jones wrote:
[ ... ]
>> Yep, this was the first thing I did as well! ;-) (*late* last night)
>>
>> I haven't had time today to look into fixing up the msvc build
>> (or a complete re-write), so I look forward to seeing your solution.
>> (do you have msvc available? - or do you want me to look at fixing
>> it? maybe in a day or two?)
>>
> Ramsay,
> I don't have msvc, so feel free to go ahead, as much as you can.
>
> I'll send a patch for the test code I have made, and put bswap.h on hold for
> a week
> (to be able to continue with t5601/connect.c)
Unfortunately, I haven't had much time to look into this.
I do have a patch (given below) that works on Linux, cygwin,
MinGW and msvc. However, the msvc build is still broken (as a
result of _other_ commits in this 'jk/pack-bitmap' branch; as
well as the use of a VLA in another commit).
So, I still have work to do! :(
Anyway, I thought I would send what I have, so you can take a look.
Note, that I don't have an big-endian machine to test this on, so
YMMV. Indeed, the *only* testing I have done is to run the test added
by this branch (t5310-pack-bitmaps.sh), which works on Linux, cygwin
and MinGW.
[Note: I have never particularly liked htons, htonl et.al., so adding
these htonll/ntohll functions doesn't thrill me! :-D For example see
this post[1], which echo's my sentiments exactly.]
HTH
ATB,
Ramsay Jones
[1] http://commandcenter.blogspot.co.uk/2012/04/byte-order-fallacy.html
-- >8 --
Subject: [PATCH] compat/bswap.h: Fix build on cygwin, MinGW and msvc
---
compat/bswap.h | 97 --
1 file changed, 68 insertions(+), 29 deletions(-)
diff --git a/compat/bswap.h b/compat/bswap.h
index ea1a9ed..c18a78e 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -17,7 +17,20 @@ static inline uint32_t default_swab32(uint32_t val)
((val & 0x00ff) << 24));
}
+static inline uint64_t default_bswap64(uint64_t val)
+{
+ return (((val & (uint64_t)0x00ffULL) << 56) |
+ ((val & (uint64_t)0xff00ULL) << 40) |
+ ((val & (uint64_t)0x00ffULL) << 24) |
+ ((val & (uint64_t)0xff00ULL) << 8) |
+ ((val & (uint64_t)0x00ffULL) >> 8) |
+ ((val & (uint64_t)0xff00ULL) >> 24) |
+ ((val & (uint64_t)0x00ffULL) >> 40) |
+ ((val & (uint64_t)0xff00ULL) >> 56));
+}
+
#undef bswap32
+#undef bswap64
#if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
@@ -32,54 +45,80 @@ static inline uint32_t git_bswap32(uint32_t x)
return result;
}
+#define bswap64 git_bswap64
+#if defined(__x86_64__)
+static inline uint64_t git_bswap64(uint64_t x)
+{
+ uint64_t result;
+ if (__builtin_constant_p(x))
+ result = default_bswap64(x);
+ else
+ __asm__("bswap %q0" : "=r" (result) : "0" (x));
+ return result;
+}
+#else
+static inline uint64_t git_bswap64(uint64_t x)
+{
+ union { uint64_t i64; uint32_t i32[2]; } tmp, result;
+ if (__builtin_constant_p(x))
+ result.i64 = default_bswap64(x);
+ else {
+ tmp.i64 = x;
+ result.i32[0] = git_bswap32(tmp.i32[1]);
+ result.i32[1] = git_bswap32(tmp.i32[0]);
+ }
+ return result.i64;
+}
+#endif
+
#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))
#include
#define bswap32(x) _byteswap_ulong(x)
+#define bswap64(x) _byteswap_uint64(x)
#endif
-#ifdef bswap32
+#if defined(bswap32)
#undef ntohl
#undef htonl
#define ntohl(x) bswap32(x)
#define htonl(x) bswap32(x)
-#ifndef __BYTE_ORDER
-# if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN)
-# define __BYTE_ORDER BYTE_ORDER
-# define __LITTLE_ENDIAN LITTLE_ENDIAN
-# define __BIG_ENDIAN BIG_ENDIAN
-# else
-# error "Cannot determine endianness"
-# endif
+#endif
+
+#if defined(bswap64)
+
+#undef ntohll
+#undef htonll
+#define ntohll(x) bswap64(x)
+#define htonll(x) bswap64(x)
+
+#else
+
+#undef ntohll
+#undef htonll
+
+#if !defined(__BYTE_ORDER)
+# if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && defined(BIG_ENDIAN)
+# define __BYTE_ORDER BYTE_ORDER
+# define __LITTLE_ENDIAN LITTLE_ENDIAN
+# define __BIG_ENDIAN BIG_ENDIAN
+# endif
+#endif
+
+#if !defined(__BYTE_ORDER)
+# error "Cannot determine endianness"
#endif
#if __BYTE_ORDER == __BIG_ENDIAN
# define ntohll(n) (n)
# define htonll(n)
Re: What's cooking in git.git (Nov 2013, #01; Fri, 1)
On 01/11/13 22:52, Junio C Hamano wrote:
> Here are the topics that have been cooking. Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.
>
[ ... ]
>
>
> * fc/transport-helper-fixes (2013-11-01) 11 commits
> - transport-helper: demote lack of "force" option to a warning
> - transport-helper: add support to delete branches
> - fast-export: add support to delete refs
> - fast-import: add support to delete refs
> - transport-helper: add support for old:new refspec
> - fast-export: add new --refspec option
> - fast-export: improve argument parsing
> - transport-helper: check for 'forced update' message
> - transport-helper: add 'force' to 'export' helpers
> - transport-helper: don't update refs in dry-run
> - transport-helper: mismerge fix
>
> Updates transport-helper, fast-import and fast-export to allow the
> ref mapping and ref deletion in a way similar to the natively
> supported transports.
>
> Will merge to 'next'.
Commit ad24a30ef ("fast-export: add new --refspec option", 31-10-2013)
causes sparse to complain:
SP builtin/fast-export.c
builtin/fast-export.c:739:55: warning: Variable length array is used.
Do we want to use this C99 feature?
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #07; Mon, 28)
On 30/10/13 20:30, Torsten Bögershausen wrote:
> On 2013-10-30 20.06, Ramsay Jones wrote:
>> On 30/10/13 17:14, Torsten Bögershausen wrote:
>>> On 2013-10-30 18.01, Vicent Martí wrote:
>>>> On Wed, Oct 30, 2013 at 5:51 PM, Torsten Bögershausen
>>>> wrote:
>>>>> There is a name clash under cygwin 1.7 (1.5 is OK)
>>>>> The following "first aid hot fix" works for me:
>>>>> /Torsten
>>>>
>>>> If Cygwin declares its own bswap_64, wouldn't it be better to use it
>>>> instead of overwriting it with our own?
>>> Yes,
>>> this will be part of a longer patch.
>>> I found that some systems have something like this:
>>>
>>> #define htobe64(x) bswap_64(x)
>>> And bswap_64 is a function, so we can not detect it by "asking"
>>> #ifdef bswap_64
>>> ..
>>> #endif
>>>
>>>
>>> But we can use
>>> #ifdef htobe64
>>> ...
>>> #endif
>>> and this will be part of a bigger patch.
>>>
>>> And, in general, we should avoid to introduce functions which may have a
>>> name clash.
>>> Using the git_ prefix for function names is a good practice.
>>> So in order to unbrake the compilation error under cygwin 17,
>>> the "hotfix" can be used.
>>
>> heh, my patch (given below) took a different approach, but
>>
>> ATB,
>> Ramsay Jones
>>
>> -- >8 --
>> Subject: [PATCH] compat/bswap.h: Fix redefinition of bswap_64 error on cygwin
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> Since commit 452e0f20 ("compat: add endianness helpers", 24-10-2013)
>> the cygwin build has failed like so:
>>
>> GIT_VERSION = 1.8.4.1.804.g1f3748b
>> * new build flags
>> CC credential-store.o
>> In file included from git-compat-util.h:305:0,
>> from cache.h:4,
>> from credential-store.c:1:
>> compat/bswap.h:67:24: error: redefinition of 'bswap_64'
>> In file included from /usr/include/endian.h:32:0,
>> from /usr/include/cygwin/types.h:21,
>> from /usr/include/sys/types.h:473,
>> from /usr/include/sys/unistd.h:9,
>> from /usr/include/unistd.h:4,
>> from git-compat-util.h:98,
>> from cache.h:4,
>> from credential-store.c:1:
>> /usr/include/byteswap.h:31:1: note: previous definition of \
>> ‘bswap_64’ was here
>> Makefile:1985: recipe for target 'credential-store.o' failed
>> make: *** [credential-store.o] Error 1
>>
>> Note that cygwin has a defintion of 'bswap_64' in the
>> header file (which had already been included by git-compat-util.h).
>> In order to suppress the error, ensure that the header
>> is included, just like the __GNUC__/__GLIBC__ case, rather than
>> attempting to define a fallback implementation.
>>
>> Signed-off-by: Ramsay Jones
>> ---
>> compat/bswap.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/compat/bswap.h b/compat/bswap.h
>> index ea1a9ed..b864abd 100644
>> --- a/compat/bswap.h
>> +++ b/compat/bswap.h
>> @@ -61,7 +61,7 @@ static inline uint32_t git_bswap32(uint32_t x)
>> # define ntohll(n) (n)
>> # define htonll(n) (n)
>> #elif __BYTE_ORDER == __LITTLE_ENDIAN
>> -# if defined(__GNUC__) && defined(__GLIBC__)
>> +# if defined(__GNUC__) && (defined(__GLIBC__) || defined(__CYGWIN__))
>> # include
>> # else /* GNUC & GLIBC */
>> static inline uint64_t bswap_64(uint64_t val)
>>
>
> Nice, much better.
>
> And here comes a patch for a big endian machine.
> I tryied to copy-paste a patch in my mail program,
> not sure if this applies.
>
> -- >8 --
> Subject: [PATCH] compat/bswap.h: htonll and ntohll for big endian
>
> Since commit 452e0f20 ("compat: add endianness helpers", 24-10-2013)
> the build on a Linux/ppc gave a warning like this:
> CC ewah/ewah_io.o
> ewah/ewah_io.c: In function ‘ewah_serialize_to’:
> ewah/ewah_io.c:81: warning: implicit declaration of function ‘htonll’
> ewah/ewah_io.c: In function ‘ewah_read_mmap’:
> ewah/ewah_io.c:132: warning: implicit declaration of function ‘ntohll’
>
> Fix it by placing the #endif for "#ifdef bswap32"
Re: What's cooking in git.git (Oct 2013, #07; Mon, 28)
On 30/10/13 17:39, Torsten Bögershausen wrote:
> On 2013-10-30 18.14, Torsten Bögershausen wrote:
>> On 2013-10-30 18.01, Vicent Martí wrote:
>>> On Wed, Oct 30, 2013 at 5:51 PM, Torsten Bögershausen wrote:
>>>> There is a name clash under cygwin 1.7 (1.5 is OK)
>>>> The following "first aid hot fix" works for me:
>>>> /Torsten
>>>
>>> If Cygwin declares its own bswap_64, wouldn't it be better to use it
>>> instead of overwriting it with our own?
>> Yes,
>> this will be part of a longer patch.
>> I found that some systems have something like this:
>>
>> #define htobe64(x) bswap_64(x)
>> And bswap_64 is a function, so we can not detect it by "asking"
>> #ifdef bswap_64
>> ..
>> #endif
>>
>>
>> But we can use
>> #ifdef htobe64
>> ...
>> #endif
>> and this will be part of a bigger patch.
>>
>> And, in general, we should avoid to introduce functions which may have a
>> name clash.
>> Using the git_ prefix for function names is a good practice.
>> So in order to unbrake the compilation error under cygwin 17,
>> the "hotfix" can be used.
>> /Torsten
> I just realized that there seem to problems to compile pu under msysgit.
> More investigation needed here.
... I noticed this too, and my patch is given below (I have another
patch for mingw which fixes some printf format warnings too) ...
However, you would not be surprised to hear that this breaks on msvc
too, so I too was planning a larger re-write ... :-D
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] compat/bswap.h: Fix failure to determine endianness on MinGW
Since commit 452e0f20 ("compat: add endianness helpers", 24-10-2013)
added the 'ntohll' and 'htonll' helpers, the MinGW build has failed
like so:
GIT_VERSION = 1.8.4.1.804.g1f3748b
* new build flags
CC credential-store.o
In file included from git-compat-util.h:305,
from cache.h:4,
from credential-store.c:1:
compat/bswap.h:56:4: error: #error "Cannot determine endianness"
make: *** [credential-store.o] Error 1
The #error is triggered because the 'endian macros' BYTE_ORDER,
LITTLE_ENDIAN and BIG_ENDIAN not being defined. On MinGW, these macros
are defined in the header file. In order to suppress the
error, set the build variable NEEDS_SYS_PARAM_H, which will cause the
"git-compat-util.h" header file to include .
Signed-off-by: Ramsay Jones
---
config.mak.uname | 1 +
1 file changed, 1 insertion(+)
diff --git a/config.mak.uname b/config.mak.uname
index 82d549e..c03ea1e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -469,6 +469,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
pathsep = ;
NO_PREAD = YesPlease
NEEDS_CRYPTO_WITH_SSL = YesPlease
+ NEEDS_SYS_PARAM_H = YesPlease
NO_LIBGEN_H = YesPlease
NO_POLL = YesPlease
NO_SYMLINK_HEAD = YesPlease
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #07; Mon, 28)
On 30/10/13 17:14, Torsten Bögershausen wrote:
> On 2013-10-30 18.01, Vicent Martí wrote:
>> On Wed, Oct 30, 2013 at 5:51 PM, Torsten Bögershausen wrote:
>>> There is a name clash under cygwin 1.7 (1.5 is OK)
>>> The following "first aid hot fix" works for me:
>>> /Torsten
>>
>> If Cygwin declares its own bswap_64, wouldn't it be better to use it
>> instead of overwriting it with our own?
> Yes,
> this will be part of a longer patch.
> I found that some systems have something like this:
>
> #define htobe64(x) bswap_64(x)
> And bswap_64 is a function, so we can not detect it by "asking"
> #ifdef bswap_64
> ..
> #endif
>
>
> But we can use
> #ifdef htobe64
> ...
> #endif
> and this will be part of a bigger patch.
>
> And, in general, we should avoid to introduce functions which may have a
> name clash.
> Using the git_ prefix for function names is a good practice.
> So in order to unbrake the compilation error under cygwin 17,
> the "hotfix" can be used.
heh, my patch (given below) took a different approach, but
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] compat/bswap.h: Fix redefinition of bswap_64 error on cygwin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Since commit 452e0f20 ("compat: add endianness helpers", 24-10-2013)
the cygwin build has failed like so:
GIT_VERSION = 1.8.4.1.804.g1f3748b
* new build flags
CC credential-store.o
In file included from git-compat-util.h:305:0,
from cache.h:4,
from credential-store.c:1:
compat/bswap.h:67:24: error: redefinition of 'bswap_64'
In file included from /usr/include/endian.h:32:0,
from /usr/include/cygwin/types.h:21,
from /usr/include/sys/types.h:473,
from /usr/include/sys/unistd.h:9,
from /usr/include/unistd.h:4,
from git-compat-util.h:98,
from cache.h:4,
from credential-store.c:1:
/usr/include/byteswap.h:31:1: note: previous definition of \
‘bswap_64’ was here
Makefile:1985: recipe for target 'credential-store.o' failed
make: *** [credential-store.o] Error 1
Note that cygwin has a defintion of 'bswap_64' in the
header file (which had already been included by git-compat-util.h).
In order to suppress the error, ensure that the header
is included, just like the __GNUC__/__GLIBC__ case, rather than
attempting to define a fallback implementation.
Signed-off-by: Ramsay Jones
---
compat/bswap.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/bswap.h b/compat/bswap.h
index ea1a9ed..b864abd 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -61,7 +61,7 @@ static inline uint32_t git_bswap32(uint32_t x)
# define ntohll(n) (n)
# define htonll(n) (n)
#elif __BYTE_ORDER == __LITTLE_ENDIAN
-# if defined(__GNUC__) && defined(__GLIBC__)
+# if defined(__GNUC__) && (defined(__GLIBC__) || defined(__CYGWIN__))
# include
# else /* GNUC & GLIBC */
static inline uint64_t bswap_64(uint64_t val)
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] http.c: Spell the null pointer as NULL
Commit 1bbcc224 ("http: refactor options to http_get_*", 28-09-2013)
changed the type of final 'options' argument of the http_get_file()
function from an int to an 'struct http_get_options' pointer.
However, it neglected to update the (single) call site. Since this
call was passing '0' to that argument, it was (correctly) being
interpreted as a null pointer. Change to argument to NULL.
Noticed by sparse. ("Using plain integer as NULL pointer")
Signed-off-by: Ramsay Jones
---
Hi Junio,
This is a repost of:
http://article.gmane.org/gmane.comp.version-control.git/236201
I suspect that this simply fell through the cracks ... (if not,
please let me know ;-)
ATB,
Ramsay Jones
http.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http.c b/http.c
index 96d7578..b133ffd 100644
--- a/http.c
+++ b/http.c
@@ -1045,7 +1045,7 @@ static char *fetch_pack_index(unsigned char *sha1, const
char *base_url)
strbuf_addf(&buf, "%s.temp", sha1_pack_index_name(sha1));
tmp = strbuf_detach(&buf, NULL);
- if (http_get_file(url, tmp, 0) != HTTP_OK) {
+ if (http_get_file(url, tmp, NULL) != HTTP_OK) {
error("Unable to get pack index %s", url);
free(tmp);
tmp = NULL;
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] http.c: Spell the null pointer as NULL
Commit 1bbcc224 ("http: refactor options to http_get_*", 28-09-2013)
changed the type of final 'options' argument of the http_get_file()
function from an int to an 'struct http_get_options' pointer.
However, it neglected to update the (single) call site. Since this
call was passing '0' to that argument, it was (correctly) being
interpreted as a null pointer. Change to argument to NULL.
Noticed by sparse. ("Using plain integer as NULL pointer")
Signed-off-by: Ramsay Jones
---
Hi Jonathan, Junio,
I'm a little puzzled by not having noticed this until this evening! ;-)
Also, I note that ma...@kernel.org != ma...@repo.or.cz/jrn
ATB,
Ramsay Jones
http.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/http.c b/http.c
index 96d7578..b133ffd 100644
--- a/http.c
+++ b/http.c
@@ -1045,7 +1045,7 @@ static char *fetch_pack_index(unsigned char *sha1, const
char *base_url)
strbuf_addf(&buf, "%s.temp", sha1_pack_index_name(sha1));
tmp = strbuf_detach(&buf, NULL);
- if (http_get_file(url, tmp, 0) != HTTP_OK) {
+ if (http_get_file(url, tmp, NULL) != HTTP_OK) {
error("Unable to get pack index %s", url);
free(tmp);
tmp = NULL;
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] miscellaneous patches
On 15/10/13 00:25, Jonathan Nieder wrote: > Ramsay Jones wrote: > >> These patches don't have too much in common, hence the subject >> line, except perhaps that 4 of them fix sparse warnings. > > Thanks. These look good. > > I tweaked the descriptions a bit to focus on what sparse was warning > about instead of our having quieted sparse. :) > Yes, the subject lines are much better. Thanks! ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] howto/setup-git-server-over-http.txt: Fix asciidoc formatting
The text contains two 'grep' invocations which include the 'start of line' regular expression character '^'. Asciidoc mis-interprets this use of '^' as a superscript request. In order to fix this formatting problem, use backticks (`) to quote the text of the affected 'grep' command invocations. Signed-off-by: Ramsay Jones --- Hi Jonathan, $ git grep '\^' Documentation/howto pointed me to some other candidates, but only this one needed a similar fix ... :-D ATB, Ramsay Jones Documentation/howto/setup-git-server-over-http.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/howto/setup-git-server-over-http.txt b/Documentation/howto/setup-git-server-over-http.txt index 7f4943e..981cbdd 100644 --- a/Documentation/howto/setup-git-server-over-http.txt +++ b/Documentation/howto/setup-git-server-over-http.txt @@ -81,8 +81,8 @@ Initialize a bare repository $ git --bare init -Change the ownership to your web-server's credentials. Use "grep ^User -httpd.conf" and "grep ^Group httpd.conf" to find out: +Change the ownership to your web-server's credentials. Use `"grep ^User +httpd.conf"` and `"grep ^Group httpd.conf"` to find out: $ chown -R www.www . -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] howto/revert-a-faulty-merge.txt: Fix asciidoc formatting
Several uses of the '^' operator are being interpreted by asciidoc as requests to show the following text as a superscript. In order to fix this problem, use backticks (`) to quote the text of the affected git command invocations. Signed-off-by: Ramsay Jones --- Hi Jonathan, Just noticed this in passing ... I don't know asciidoc that well, so if there is a better way to fix this, just let me know. ;-) ATB, Ramsay Jones Documentation/howto/revert-a-faulty-merge.txt | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/howto/revert-a-faulty-merge.txt b/Documentation/howto/revert-a-faulty-merge.txt index 075418e..acf3e47 100644 --- a/Documentation/howto/revert-a-faulty-merge.txt +++ b/Documentation/howto/revert-a-faulty-merge.txt @@ -37,7 +37,7 @@ where A and B are on the side development that was not so good, M is the merge that brings these premature changes into the mainline, x are changes unrelated to what the side branch did and already made on the mainline, and W is the "revert of the merge M" (doesn't W look M upside down?). -IOW, "diff W^..W" is similar to "diff -R M^..M". +IOW, `"diff W^..W"` is similar to `"diff -R M^..M"`. Such a "revert" of a merge can be made with: @@ -121,9 +121,9 @@ If you reverted the revert in such a case as in the previous example: ---A---B A'--B'--C' where Y is the revert of W, A' and B' are rerolled A and B, and there may -also be a further fix-up C' on the side branch. "diff Y^..Y" is similar -to "diff -R W^..W" (which in turn means it is similar to "diff M^..M"), -and "diff A'^..C'" by definition would be similar but different from that, +also be a further fix-up C' on the side branch. `"diff Y^..Y"` is similar +to `"diff -R W^..W"` (which in turn means it is similar to `"diff M^..M"`), +and `"diff A'^..C'"` by definition would be similar but different from that, because it is a rerolled series of the earlier change. There will be a lot of overlapping changes that result in conflicts. So do not do "revert of revert" blindly without thinking.. -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] sparse: Fix some "using sizeof on a function" warnings
Sparse issues an "using sizeof on a function" warning for each call to curl_easy_setopt() which sets an option that takes a function pointer parameter. (currently 12 such warnings over 4 files.) The warnings relate to the use of the "typecheck-gcc.h" header file which adds a layer of type-checking macros to the curl function invocations (for gcc >= 4.3 and !__cplusplus). As part of the type-checking layer, 'sizeof' is applied to the function parameter of curl_easy_setopt(). Note that, in the context of sizeof, the function to function pointer conversion is not performed and that sizeof(f) != sizeof(&f). A simple solution, therefore, would be to replace the function name in each such call to curl_easy_setopt() with an explicit function pointer expression (i.e. replace f with &f). However, the "typecheck-gcc.h" header file is only conditionally included, in addition to the gcc and C++ checks mentioned above, depending on the CURL_DISABLE_TYPECHECK preprocessor variable. In order to suppress the warnings, we use target-specific variable assignments to add -DCURL_DISABLE_TYPECHECK to SPARSE_FLAGS for each file affected (http-push.c, http.c, http-walker.c and remote-curl.c). Signed-off-by: Ramsay Jones --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index de3d72c..4fde227 100644 --- a/Makefile +++ b/Makefile @@ -2018,6 +2018,9 @@ gettext.sp gettext.s gettext.o: GIT-PREFIX gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \ -DGIT_LOCALE_PATH='"$(localedir_SQ)"' +http-push.sp http.sp http-walker.sp remote-curl.sp: SPARSE_FLAGS += \ + -DCURL_DISABLE_TYPECHECK + ifdef NO_EXPAT http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT endif -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] t9500-*.sh: Fix highlight test hang on Linux Mint
Linux Mint has an implementation of the highlight command (unrelated to the one from http://www.andre-simon.de) that works as a simple filter. The script uses 'sed' to add terminal colour escape codes around text matching a regular expression. When t9500-*.sh attempts to run "highlight --version", the script simply hangs waiting for input. (See https://bugs.launchpad.net/linuxmint/+bug/815005). The tool required by gitweb can be installed from the 'highlight' package. Unfortunately, given the default $PATH, this leads to the tool having lower precedence than the script. In order to avoid hanging the test, add ' --- t/t9500-gitweb-standalone-no-errors.sh | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh index 6fca193..718014d 100755 --- a/t/t9500-gitweb-standalone-no-errors.sh +++ b/t/t9500-gitweb-standalone-no-errors.sh @@ -683,9 +683,11 @@ test_expect_success \ # syntax highlighting -highlight --version >/dev/null 2>&1 +highlight_version=$(highlight --version /dev/null) if [ $? -eq 127 ]; then - say "Skipping syntax highlighting test, because 'highlight' was not found" + say "Skipping syntax highlighting tests: 'highlight' not found" +elif test -z "$highlight_version"; then + say "Skipping syntax highlighting tests: incorrect 'highlight' found" else test_set_prereq HIGHLIGHT cat >>gitweb_config.perl <<-\EOF -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] git-format-patch.txt: Add to Thunderbird configuration
The Thunderbird section of the 'MUA-specific hints' contains three different approaches to setting up the mail client to leave patch emails unmolested. The second approach (configuration) has a step missing when configuring the composition window not to wrap. In particular, the "mailnews.wraplength" configuration variable needs to be set to zero. Update the documentation to add the missing setting. Signed-off-by: Ramsay Jones --- Documentation/git-format-patch.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 9e0ef0e..5c0a4ab 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -438,7 +438,8 @@ Edit..Preferences..Composition, wrap plain text messages at 0 In Thunderbird 3: Edit..Preferences..Advanced..Config Editor. Search for "mail.wrap_long_lines". -Toggle it to make sure it is set to `false`. +Toggle it to make sure it is set to `false`. Also, search for +"mailnews.wraplength" and set the value to 0. 3. Disable the use of format=flowed: Edit..Preferences..Advanced..Config Editor. Search for -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] wrapper.c: Fix a sparse warning
When the NO_MKSTEMPS build variable is not set, sparse issues an
"'gitmkstemps' was not declared. Should it be static?" warning.
The 'gitmkstemps' function definition is only required when the
NO_MKSTEMPS variable is set. In order to suppress the warning,
use a preprocessor conditional to include the definition only
when needed.
Signed-off-by: Ramsay Jones
---
wrapper.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/wrapper.c b/wrapper.c
index f92b147..9a6aaaf 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -360,10 +360,12 @@ int git_mkstemp_mode(char *pattern, int mode)
return git_mkstemps_mode(pattern, 0, mode);
}
+#ifdef NO_MKSTEMPS
int gitmkstemps(char *pattern, int suffix_len)
{
return git_mkstemps_mode(pattern, suffix_len, 0600);
}
+#endif
int xmkstemp_mode(char *template, int mode)
{
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] refs.c: Fix a sparse warning
Sparse issues an "Using plain integer as NULL pointer" warning
against a call to update_ref_lock() which passes '0' to the
'int *type_p' parameter. In order to suppress the warning, we
simply change the argument to 'NULL'.
Signed-off-by: Ramsay Jones
---
refs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index ad5d66c..3710748 100644
--- a/refs.c
+++ b/refs.c
@@ -3235,7 +3235,7 @@ int update_ref(const char *action, const char *refname,
int flags, enum action_on_err onerr)
{
struct ref_lock *lock;
- lock = update_ref_lock(refname, oldval, flags, 0, onerr);
+ lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
if (!lock)
return 1;
return update_ref_write(action, refname, sha1, lock, onerr);
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] config.c: Fix a sparse warning
Sparse issues an "'git_parse_unsigned' was not declared. Should it
be static?" warning. In order to suppress this warning, since this
symbol only requires file scope, we simply add the static modifier
to its declaration.
Signed-off-by: Ramsay Jones
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 6588cf5..e1d66a1 100644
--- a/config.c
+++ b/config.c
@@ -498,7 +498,7 @@ static int git_parse_signed(const char *value, intmax_t
*ret, intmax_t max)
return 0;
}
-int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
+static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
{
if (value && *value) {
char *end;
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] miscellaneous patches
Hi Jonathan, These patches don't have too much in common, hence the subject line, except perhaps that 4 of them fix sparse warnings. Note that the fourth patch is actually a simplified version of an earlier RFC patch. Junio didn't like the idea of using a build variable (GIT_TEST_HIGHLIGHT_BIN) to set the path to the 'correct' highlight tool. see: http://article.gmane.org/gmane.comp.version-control.git/234138 Having recently had to re-configure Thunderbird, I noticed that the documentation needed updating. (actually, I was sure that I had sent a similar patch, years ago, when I last did this! :-P ) ATB, Ramsay Jones Ramsay Jones (6): config.c: Fix a sparse warning refs.c: Fix a sparse warning wrapper.c: Fix a sparse warning t9500-*.sh: Fix highlight test hang on Linux Mint git-format-patch.txt: Add to Thunderbird configuration sparse: Fix some "using sizeof on a function" warnings Documentation/git-format-patch.txt | 3 ++- Makefile | 3 +++ config.c | 2 +- refs.c | 2 +- t/t9500-gitweb-standalone-no-errors.sh | 6 -- wrapper.c | 2 ++ 6 files changed, 13 insertions(+), 5 deletions(-) -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: suppress false positive warnings of empty format string.
On 29/09/13 16:07, Felipe Contreras wrote: > On Sun, Sep 29, 2013 at 7:08 AM, Stefan Beller > wrote: >> Signed-off-by: Stefan Beller >> --- >> Makefile | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/Makefile b/Makefile >> index de3d72c..60afa51 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -349,7 +349,7 @@ GIT-VERSION-FILE: FORCE >> >> # CFLAGS and LDFLAGS are for the users to override from the command line. >> >> -CFLAGS = -g -O2 -Wall >> +CFLAGS = -g -O2 -Wall -Wno-format-zero-length > > Oh yes please. > > However, somebody mentioned that this might break compilers other than > gcc, but perhaps we can do what Linux does: I simply added: CFLAGS+=-Wno-format-zero-length to my config.mak file. I had originally intended to do so conditionally, depending on the compiler being gcc, but I found that clang and tcc just ignored it ... > cc-disable-warning = $(call try-run,\ > $(CC) $(CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip > $(1))) > > CFLAGS = -g -O2 -Wall $(call cc-disable-warning,format-zero-length,) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] t9500-*.sh: Fix highlight test hang on Linux Mint
Linux Mint has an implementation of the highlight command (unrelated
to the one from http://www.andre-simon.de) that works as a simple
filter. The script uses 'sed' to add terminal colour escape codes
around text matching a regular expression. When t9500-*.sh attempts
to run "highlight --version", the script simply hangs waiting for
input. (See https://bugs.launchpad.net/linuxmint/+bug/815005).
The tool required by gitweb can be installed from the 'highlight'
package. Unfortunately, given the default $PATH, this leads to the
tool having lower precedence than the script.
In order to allow the user to specify the correct tool, introduce
the GIT_TEST_HIGHLIGHT_BIN variable. Also, add '
---
Hi Junio,
I recently upgraded my Ubuntu installation to Linux Mint 15 Cinnamon.
(Unity makes me want to throw my laptop at the wall!)
Having tickled this bug, I solved the problem by building highlight
v3.15 from source and installing in $HOME.
This patch is marked RFC because this bug does not seem to have
affected too many people (given that Heiko reported the problem
back in 2011) ... :-D
[Also, note that I didn't fix up the form of the conditional.]
ATB,
Ramsay Jones
t/t9500-gitweb-standalone-no-errors.sh | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/t/t9500-gitweb-standalone-no-errors.sh
b/t/t9500-gitweb-standalone-no-errors.sh
index 6fca193..0208c8e 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -683,14 +683,18 @@ test_expect_success \
# syntax highlighting
-highlight --version >/dev/null 2>&1
+GIT_TEST_HIGHLIGHT_BIN=${GIT_TEST_HIGHLIGHT_BIN:-highlight}
+highlight_version=$($GIT_TEST_HIGHLIGHT_BIN --version /dev/null)
if [ $? -eq 127 ]; then
- say "Skipping syntax highlighting test, because 'highlight' was not
found"
+ say "Skipping syntax highlighting tests: 'highlight' not found"
+elif test -z "$highlight_version"; then
+ say "Skipping syntax highlighting tests: incorrect 'highlight' found"
+ say "set GIT_TEST_HIGHLIGHT_BIN to full path of highlight program"
else
test_set_prereq HIGHLIGHT
- cat >>gitweb_config.perl <<-\EOF
- our $highlight_bin = "highlight";
- $feature{'highlight'}{'override'} = 1;
+ cat >>gitweb_config.perl <<-EOF
+ our \$highlight_bin = "$GIT_TEST_HIGHLIGHT_BIN";
+ \$feature{'highlight'}{'override'} = 1;
EOF
fi
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] builtin/fetch.c: Fix a sparse warning
Sparse issues an "'prepare_transport' was not declared. Should it
be static?" warning. In order to suppress the warning, since this
symbol only requires file scope, we simply add the static modifier
to it's declaration.
Signed-off-by: Ramsay Jones
---
Hi Junio,
When you re-build the next branch, could you please squash this into
commit db5723c6 ("fetch: refactor code that prepares a transport",
07-08-2013). [from the 'jc/transport-do-not-use-connect-twice-in-fetch'
branch]
Thanks!
ATB,
Ramsay Jones
builtin/fetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e880116..9e654ef 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -747,7 +747,7 @@ static void set_option(struct transport *transport, const
char *name, const char
name, transport->url);
}
-struct transport *prepare_transport(struct remote *remote)
+static struct transport *prepare_transport(struct remote *remote)
{
struct transport *transport;
transport = transport_get(remote, NULL);
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 06/24] read-cache: Don't compare uid, gid and ino on cygwin
On 18/08/2013 08:41 PM, Thomas Gummerer wrote:
> Cygwin doesn't have uid, gid and ino stats fields. Therefore we should
> never check them in the match_stat_data when working on the CYGWIN
> platform.
Hmm, this is simply not true ... ;-)
The need to omit the uid, gid and ino fields from the stat checks in
your original code was caused by the "schizophrenic stat" implementation
in cygwin. (This was also before "core.checkstat" was implemented; note
the 'check_stat' conditional below ...)
However, since commit f66450ae ("cygwin: Remove the Win32 l/stat()
implementation", 22-06-2013), this patch is no longer necessary and
can simply be dropped from this series.
[I have not had time to read your new patches yet, but I seem to remember
being concerned about those platforms which have UNRELIABLE_FSTAT set.
(ie cygwin, MinGW and Windows.)]
ATB,
Ramsay Jones
> Signed-off-by: Thomas Gummerer
> ---
>
> This patch was not tested on Cygwin yet. I think it's needed though,
> because the re-reading of the index if it changed will no longer use
> it's own index_changed function, but use the stat_validity_check
> function instead. Would be great if someone running Cygwin could test
> this.
>
> read-cache.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/read-cache.c b/read-cache.c
> index 1f827de..aa17ce7 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -82,6 +82,7 @@ int match_stat_data(const struct stat_data *sd, struct stat
> *st)
> changed |= CTIME_CHANGED;
> #endif
>
> +#if !defined (__CYGWIN__)
> if (check_stat) {
> if (sd->sd_uid != (unsigned int) st->st_uid ||
> sd->sd_gid != (unsigned int) st->st_gid)
> @@ -89,6 +90,7 @@ int match_stat_data(const struct stat_data *sd, struct stat
> *st)
> if (sd->sd_ino != (unsigned int) st->st_ino)
> changed |= INODE_CHANGED;
> }
> +#endif
>
> #ifdef USE_STDEV
> /*
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] submodule: fix confusing variable name
Fredrik Gustafsson wrote: > On Sun, Aug 04, 2013 at 07:34:48PM +0200, Jens Lehmann wrote: >> But we'll have to use sm_path here (like everywhere else in the >> submodule script), because we'll run into problems under Windows >> otherwise (see 64394e3ae9 for details). Apart from that the patch >> is fine. > > We're still using path= in the foreach-script. Or rather, we're setting > it. From what I can see and from the commit message 64394e3ae9 it could > possible be a problem. Please do not use a $path variable in any script intended to be run on windows; those poor souls who would otherwise have to fix the bugs will thank you! :-D Actually, it's not so much the use of a $path variable, rather the act of _exporting_ such a variable that causes the problem. (Which is why using $path with eval_gettext[ln] is such a problem, of course.) As noted in the above commit, $path is unfortunately a documented part of the public API for the foreach subcommand. However, the foreach subcommand is (mostly) fine; given the fact that the user script is eval-ed in a context in which $path is not exported. The reason for the 'mostly' is simply that the user could shoot himself in the foot by export-ing $path in their script, so that something like: $ git submodule foreach 'export path; echo $path `git rev-parse HEAD`' will indeed fail (ie git rev-parse will not execute). > Not sure how to solve it though... Just a simple correction would break > all script depending on that. $path is part of the public API, so we can't just remove it. It would require a deprecation period, etc,. (Adding/documenting $sm_path as an alternative *may* be worth doing. dunno.) HTH ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2013, #09; Mon, 29)
Junio C Hamano wrote: > Ramsay Jones writes: > >>> I am personally in favor of this simpler solution. Comments? >> >> I had expected this to me marked for 'master'. >> >> Has this simply been overlooked, or do you have reservations about >> applying this patch? > > I am just being careful and do want to keep it cooking in 'next' > during the feature freeze. The more users work with 'next' (not > "work *on* 'next'"), the more confidence we would be with, and > hopefully this can be one of the topis that graduate early after > the 1.8.4 release. Hmm, this patch is a bug-fix for a bug that (currently) will be _introduced_ by v1.8.4. Do you want me to try and find a different bug-fix for v1.8.4? (Although that would most likely be more risky than simply taking this patch! ;-) ). ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] "git config --get-urlmatch $section.$key $url"
Junio C Hamano wrote: > So here is a bit of refactoring to extract the logic to find the > entry $section..$key from the configuration that best > matches the given $url to answer "what value $section.$key should be > for $url?" out of http.c (primarily because we never want to link > "git cofnig" with the cURL library), and use it from "git config" to > implement Peff's idea to extend "git config". > > The first step is a pure code movement, plus some renaming of the > functions. > > The second step is to factor out the code to handle --bool, --int, etc. > as a helper so that the new codepath can use it. > > The last step currently duplicates the logic in http_options(), but > we might want to refactor it further so that the two functions can > share more code. We hopefully can get rid of test-url-normalize and > instead use "git config --get-urlmatch" in the tests that protect > the http..config topic. I haven't been following this topic too closely and I don't have any feel for how long it will take to get to the end-game. However, unless the removal of test-url-normalize is coming soon, could I request that you apply my patch (or squash it into this series)? At present, I have to apply the patch before building the next and pu branches; OK it's not too onerous, but still ... :-P ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2013, #09; Mon, 29)
Junio C Hamano wrote: > Here are the topics that have been cooking. Commits prefixed with > '-' are only in 'pu' (proposed updates) while commits prefixed with > '+' are in 'next'. > > The shape of the upcoming release is pretty much known by now; all > the topics that are marked for 'master' in this issue will likely to > be in the final, and others will cook during the pre-release freeze > period. > [ ... ] > > * rj/cygwin-clarify-use-of-cheating-lstat (2013-07-18) 1 commit > - cygwin: Remove the Win32 l/stat() implementation > > Cygwin port added a "not quite correct but a lot faster and good > enough for many lstat() calls that are only used to see if the > working tree entity matches the index entry" lstat() emulation some > time ago, and it started biting us in places. This removes it and > uses the standard lstat() that comes with Cygwin. > > I am personally in favor of this simpler solution. Comments? I had expected this to me marked for 'master'. Has this simply been overlooked, or do you have reservations about applying this patch? If so, then I need to find another solution very quickly [1] (before v1.8.4). At this time, despite passing the test suite [2], the cygwin build is still very much broken. ATB, Ramsay Jones [1] I do have another patch, patch #0 actually, which I said I didn't want applied! :-P [2] I ran the test suite on v1.8.4-rc0 + 1 patch, like so: $ GIT_SKIP_TESTS='t0061.3' make test >test-outp16 2>&1 $ tail -17 test-outp16 [22:33:53] t9902-completion.sh ok13650 ms [22:34:26] t9903-bash-prompt.sh ... ok33468 ms [22:34:26] All tests successful. Test Summary Report --- t7008-grep-binary.sh (Wstat: 0 Tests: 23 Failed: 0) TODO passed: 12 Files=639, Tests=10291, 9581 wallclock secs ( 2.75 usr 1.41 sys + 9421.84 cusr 4551.31 csys = 13977.31 CPU) Result: PASS make clean-except-prove-cache make[2]: Entering directory `/home/ramsay/git/t' rm -f -r 'trash directory'.* 'test-results' rm -f -r valgrind/bin make[2]: Leaving directory `/home/ramsay/git/t' make[1]: Leaving directory `/home/ramsay/git/t' $ This was over 30 minutes faster than the last full test suite run, but it also ran more tests (420 test files ran faster and 16 new test files were run). I've *never* had the test suite run faster before! (I'm not going to celebrate too much; it still took 9581s = 2h39m41s). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] revision.c: Fix a sparse warning
Sparse issues an "symbol 'saved_parents_slab' was not declared. Should
it be static?" warning. In order to suppress the warning, since this
symbol does not require more than file visibility, we simply add the
static modifier to its declaration.
Signed-off-by: Ramsay Jones
---
Hi Thomas,
In addition to the gcc warning, sparse weighs in with this warning,
provoked by commit 3b3d83e5 ("[PERHAPS LIKE THIS] log: use true parents
for diff even when rewriting", 22-07-2013).
If you update this commit, could you please squash this into the new patch.
Thanks!
ATB,
Ramsay Jones
revision.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/revision.c b/revision.c
index f242363..fa355d0 100644
--- a/revision.c
+++ b/revision.c
@@ -3074,7 +3074,7 @@ void put_revision_mark(const struct rev_info *revs, const
struct commit *commit)
}
define_commit_slab(saved_parents, struct commit_list *);
-struct saved_parents saved_parents_slab;
+static struct saved_parents saved_parents_slab;
static int saved_parents_initialized;
void save_parents(struct commit *commit)
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] commit-slab.h: Fix memory allocation and addressing
The slab initialization code includes the calculation of the
slab 'elem_size', which is in turn used to determine the size
(capacity) of the slab. Each element of the slab represents an
array, of length 'stride', of 'elemtype'. (Note that it may be
clearer if the define_commit_slab macro parameter was called
'basetype' rather than 'elemtype'). However, the 'elem_size'
calculation incorrectly uses 'sizeof(struct slabname)' in the
expression, rather than 'sizeof(elemtype)'.
Within the slab access routine, _at(), the given commit
'index' is transformed into an (slab#, slot#) pair used to address
the required element (a pointer to the first element of the array
of 'elemtype' associated with that commit). The current code to
calculate these address coordinates multiplies the commit index
by the 'stride' which, at least for the slab#, produces the wrong
result. Using the commit index directly, without scaling by the
'stride', produces the correct 'logical' address.
Also, when allocating a new slab, the size of the allocation only
allows for a slab containing elements of single element arrays of
'elemtype'. This should allow for elements of an array of length
'stride' of 'elemtype'. In order to fix this, we need to change
the element size parameter to xcalloc() by multiplying the current
element size (sizeof(**s->slab)) by the s->stride.
Having changed the calculation of the slot#, we now need to convert
the logical 'nth_slot', by scaling with s->stride, into the correct
physical address.
Signed-off-by: Ramsay Jones
---
Hi Junio,
While looking into a sparse warning, which involved the use of the
"commit-slab.h" header file, I noticed some problems with that code.
(at least I _think_ I did! ;-)
I was convinced, just by reading the code in the header, that when
used with stride > 1, the memory allocated to a slab would not be
sufficient. (ie it would be too small by:
s->slab_size * (sizeof(**s->slab) * (stride - 1))
). So, I had expected t3202-show-branch-octopus.sh to provoke memory
error reports when run under valgrind.
Hmm, it didn't ... so much for that theory! :-D
So, I'm a little puzzled; I must be missing something obvious, which
is why this is marked RFC.
What am I missing?
ATB,
Ramsay Jones
commit-slab.h | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/commit-slab.h b/commit-slab.h
index 7d48163..d4c8286 100644
--- a/commit-slab.h
+++ b/commit-slab.h
@@ -48,7 +48,7 @@ static void init_ ##slabname## _with_stride(struct slabname
*s, \
if (!stride)\
stride = 1; \
s->stride = stride; \
- elem_size = sizeof(struct slabname) * stride; \
+ elem_size = sizeof(elemtype) * stride; \
s->slab_size = COMMIT_SLAB_SIZE / elem_size;\
s->slab_count = 0; \
s->slab = NULL; \
@@ -72,11 +72,10 @@ static void clear_ ##slabname(struct slabname *s)
\
static elemtype *slabname## _at(struct slabname *s,\
const struct commit *c) \
{ \
- int nth_slab, nth_slot, ix; \
+ int nth_slab, nth_slot; \
\
- ix = c->index * s->stride; \
- nth_slab = ix / s->slab_size; \
- nth_slot = ix % s->slab_size; \
+ nth_slab = c->index / s->slab_size; \
+ nth_slot = c->index % s->slab_size; \
\
if (s->slab_count <= nth_slab) {\
int i; \
@@ -89,8 +88,8 @@ static elemtype *slabname## _at(struct slabname *s,
\
} \
if (!s->slab[nth_slab]) \
s->slab[nth_slab] = xcalloc(s->slab_size, \
-
Re: What's cooking in git.git (Jul 2013, #07; Sun, 21)
Torsten Bögershausen wrote: > >> ml/cygwin-updates: >> cygwin: stop forcing core.filemode=false > > I like that: cygwin behaves more like Unix/Linux. > > Just a side-comment: When working on NTFS, cygwin > will set core.filemode=true, and as a result of that, > the "cheating lstat" code is not used any more. > > So it is not run under the test suite (typically NTFS), > and therefore "untested by default". Indeed, the next branch is now "fixed". :-D > >> * rj/cygwin-clarify-use-of-cheating-lstat (2013-07-18) 1 commit >> - cygwin: Remove the Win32 l/stat() implementation > >> I am personally in favor of this simpler solution. Comments? > Me too, thanks to all contributors Thank you for taking the time to help address this issue! ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] test-url-normalize.c: Fix gcc errors and sparse warnings
Sparse issues an "non-ANSI function declaration of function 'main'"
warning when NO_CURL is set. In order to suppress the warning, we
simply add the function prototype.
When NO_CURL and USE_CURL_MULTI are not defined, then gcc issues the
following error:
CC test-url-normalize.o
test-url-normalize.c: In function `run_http_options':
test-url-normalize.c:49: error: `max_requests' undeclared (first use in this
function)
test-url-normalize.c:49: error: (Each undeclared identifier is reported only
once
test-url-normalize.c:49: error: for each function it appears in.)
make: *** [test-url-normalize.o] Error 1
In order to fix the error, we simply protect the use of the 'max_requests'
variable with an preprocessor conditional.
Signed-off-by: Ramsay Jones
---
Hi Kyle,
When you next update the patches in your 'km/http-curl-config-per-url'
branch, could you please squash this (or something like it) into the
relevant patches.
Thanks!
ATB,
Ramsay Jones
test-url-normalize.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/test-url-normalize.c b/test-url-normalize.c
index abfa4eb..86ce553 100644
--- a/test-url-normalize.c
+++ b/test-url-normalize.c
@@ -1,6 +1,6 @@
#ifdef NO_CURL
-int main()
+int main(void)
{
return 125;
}
@@ -45,8 +45,10 @@ static int run_http_options(const char *file,
printf("%s\n", curl_ssl_try ? "true" : "false");
else if (!strcmp("minsessions", opt_lc.buf))
printf("%d\n", min_curl_sessions);
+#ifdef USE_CURL_MULTI
else if (!strcmp("maxrequests", opt_lc.buf))
printf("%d\n", max_requests);
+#endif
else if (!strcmp("lowspeedlimit", opt_lc.buf))
printf("%ld\n", curl_low_speed_limit);
else if (!strcmp("lowspeedtime", opt_lc.buf))
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2013, #03; Tue, 9)
Junio C Hamano wrote: > Ramsay Jones writes: > >> Junio C Hamano wrote: >> [ ... ] >>> * rr/send-email-ssl-verify (2013-07-06) 6 commits >>> - SQUASH??? update to support SSL_ca_file as well as SSL_ca_path >>> - SQUASH??? send-email: cover both smtps and starttls cases >>> - fixup! send-email: squelch warning from Net::SMTP::SSL >>> - SQUASH??? send-email giving default value to ssl-cert-path with ||= >>> assignment >>> - send-email: introduce sendemail.smtpsslcertpath >>> - send-email: squelch warning from Net::SMTP::SSL >>> >>> The issue seems a lot deeper than the initial attempt and needs >>> somebody to sit down and sort out to get the version dependencies >>> and lazy loading right. >> >> This causes test failures for me, since send-email can't load the >> IO/Socket/SSL.pm module. (on Linux, cygwin and MinGW!) > > Good ;-). > > Can you try the more recent 'pu' with 35035bbf (send-email: be > explicit with SSL certificate verification, 2013-07-18) that was > updated with help from Brian Carlson? Yes, this works fine now. I tested on Linux and cygwin. (I haven't had time to test on MinGW, but I don't expect any problems.) Thanks ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Cygwin has trustable filemode
Mark Levedahl wrote:
> The supported Cygwin distribution on supported Windows versions provides
> complete support for POSIX filemodes, so enable this by default. git as
> distributed by the Cygwin project is configured this way.
>
> This fixes one testsuite failure:
> t3300 test 17 (diff-index -M -p with mode change quotes funny filename)
>
> Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus
> dropped support for all OS variants that lack NTFS and/or the full win32
> api, and since late 1.5 development, Cygwin maps POSIX modes to NTFS ACLs
> by default. Cygwin 1.5 supported OS variants that used FAT as the native
> file system, and had optional methods for providing POSIX file modes on
> top of FAT12/16 and NTFS, though not FAT32. Also, support for POSIX modes
> on top of FAT were dropped later in 1.5. Thus, POSIX filemode support
> could not be expected by default on a Cygwin 1.5 installation, but is
> expected by default on a 1.7 installation.
>
> Signed-off-by: Mark Levedahl
> ---
> Junio - The above notes are more accurate than in my previous commit message,
> so if this commit survives into next/master, I would prefer this version as
> opposed to the one now on pu (da875762)
Again, I have to ask; should you not "revert" commit c869753e ("Force
core.filemode
to false on Cygwin.", 30-12-2006)? After this commit, there is no longer any
user
of the NO_TRUSTABLE_FILEMODE build variable, and no real prospect of anyone else
wanting to use it.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add the GIT_SENTINEL macro
Junio C Hamano wrote:
> Ramsay Jones writes:
>
>> The sentinel function attribute is not understood by versions of
>> the gcc compiler prior to v4.0. At present, for earlier versions
>> of gcc, the build issues 108 warnings related to the unknown
>> attribute. In order to suppress the warnings, we conditionally
>> define the GIT_SENTINEL macro to provide the sentinel attribute
>> for gcc v4.0 and newer.
>>
>> Signed-off-by: Ramsay Jones
>> ---
>>
>> This was built on the next branch
>
> It seems to apply well on the tip of jk/gcc-function-attributes.
>
>
> - This macro is not about "git" at all, so I'll edit the patch to
>call it GCC_ATTR_SENTINEL before applying.
>
> - Also I'll drop the (0) at the end.
>
> Thanks.
Yes, GCC_ATTR_SENTINEL is a better name, but I like LAST_ARG_MUST_BE_NULL
even more! The final commit 9fe3edc4 ("Add the LAST_ARG_MUST_BE_NULL macro",
18-07-2013) is much better than my patch and works beautifully.
Thanks Junio, Jeff and Jonathan!
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix some sparse warnings
Jeff King wrote:
> On Thu, Jul 18, 2013 at 09:25:50PM +0100, Ramsay Jones wrote:
>
>> Sparse issues some "Using plain integer as NULL pointer" warnings.
>> Each warning relates to the use of an '{0}' initialiser expression
>> in the declaration of an 'struct object_info'. The first field of
>> this structure has pointer type. Thus, in order to suppress these
>> warnings, we replace the initialiser expression with '{NULL}'.
>>
>> Signed-off-by: Ramsay Jones
>
> Acked-by: Jeff King
>
> I thought at first we would need one more for the new callsite I added
> in my series, but we use memset() in that instance, so it is fine.
On an almost unrelated note ... I am now getting the following sparse
warnings:
pack-revindex.c:105:23: warning: memset with byte count of 262144
This is a little annoying, since there is no way to turn this off. :(
(which I consider a bug in sparse).
Sparse has special-case code to check calls to memset(), memcpy(),
copy_to_user() and copy_from_user(). The code that checks the byte
count argument looks like:
static void check_byte_count(struct instruction *insn, pseudo_t count)
{
if (!count)
return;
if (count->type == PSEUDO_VAL) {
long long val = count->value;
if (val <= 0 || val > 10)
warning(insn->pos, "%s with byte count of %lld",
show_ident(insn->func->sym->ident), val);
return;
}
/* OK, we could try to do the range analysis here */
}
I will just ignore this for now. ;-)
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Add the NO_SENTINEL build variable
Junio C Hamano wrote: > Ramsay Jones writes: > >> Jonathan Nieder wrote: >>> Ramsay Jones wrote: >>> >>>> One of the three gcc compilers that I use does not understand the >>>> sentinel function attribute. (so, it spews 108 warning messages) >>> >>> Do you know what version of gcc introduced the sentinel attribute? >>> Would it make sense for the ifdef in git-compat-util.h to be keyed on >>> __GNUC__ and __GNUC_MINOR__ instead of a new makefile flag? >>> >> >> I have on old (v4.2.1) gcc repo on Linux and looking at >> >> ~/gcc-4.2.1/gcc/ChangeLog-2004 >> >> I can see that the sentinel attribute was added on 2004-09-04 by >> Kaveh R. Ghazi. >> >> Also, I find "bump version string to version 4.0.0" was on 2004-09-09 >> and "bump version string to version 3.5.0" was on 2004-01-16. >> >> Several of my system header files (on Linux) imply that the >> sentinel attribute is supported by __GNUC__ >= 4. (One of them, >> ansidecl.h, states that gcc 3.5 supports it but ...) > > Perhaps a message from yesterday would have helped? > > http://article.gmane.org/gmane.comp.version-control.git/230633 > > seems to indicate that checking for version 4 is sufficient. > > Also I asked you to split the __attribute__((sentinel(n)) support > into a separate patch. We currently do not pass anything but 0 > (meaning, the sentinel is always at the end), and SENTINEL in all > capital is easy enough to grep for when somebody _does_ want to have > such a support, so I'd prefer not to see __attribute__((sentinel(n)) > until it becomes necessary. Sorry, but I didn't see any of these emails before sending this and the subsequent patch email. :( I can sometimes be away from email for several days at a time (and then spend days trying to read the backlog - I'm on *far* too many mailing lists!). [The internet went black on me last night; I couldn't see anything outside of my ISP's servers (and not all of those). I also couldn't get any answer at the support phone number, so I probably wasn't the only one ...] ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Cygwin has trustable filemode
Mark Levedahl wrote: > On 07/19/2013 12:40 PM, Junio C Hamano wrote: >> Thanks, will replace. >> >> What do we want to do with the compat/regex build-time switch? >> >> IIRC, this was only needed for 1.7 and not 1.5, and I also would >> expect (without anything to back-up, so this is more a faith than >> expectation) over time the "new library" would have a working regex >> library. >> > > The situation is that Cygwin uses newlib rather than glibc, and does so > for licesnsing reasons (redhat sells licenses to developers allowing > closed source applications built using Cygwin). So, there must be a > compelling need to fix the library - git has a simple work around, so > isn't the case. Also, Cygwin has a perl regex library for those > demanding more complete / correct regex solution. So, I make no > prediction on when the newlib regex functions are fixed. > > Related: Should we have separate settings for 1.5 and 1.7 for several > variables? We already do. > Conflicts I see not reflected in config.mak.uname on pu: > trustable filemode (1.7 has, 1.5 does not) I see no need for any difference here. puzzled. > MMAP/Pread (1.7 pread is thread safe, 1.5 I dont think was, MMAP > utility is convolved in this) pread() is now thread-safe? great! (It must have been a fairly recent change; last time I looked it was still not thread-safe on 1.7.) > regex - 1.7 is broken, per Ramsay 1.5 works I don't see any reason not to use the compat/regex routines on both cygwin 1.5 and 1.7. However, I wouldn't object to restricting the use of the compat routines to cygwin 1.7 either! > If you think its worth it, I'll create a patch series with the above and > justifications for the different settings that I know. As far as I can see, only the pread() and maybe MMAP and regex setting need to change from the current setup. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test-lib.sh - define and use GREP_STRIPS_CR
Mark Levedahl wrote: > Define a common macro for grep needing -U to allow tests to not need > to inquire of specific platforms needing this option. Change > t3032 and t5560 to use this rather than testing explicitly for mingw. > This fixes these two tests on Cygwin. > > Signed-off-by: Mark Levedahl > --- > This replaces my earlier patch against t3032 (8896b287 on pu) Yep, this looks good and (as expected) it works on cygwin 1.5 too. :-D Thanks. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t3032 incompatible with Cygwin/Windows
Mark Levedahl wrote: > Subtests 6,7, and 9 of t3032 fail on Cygwin, and I presume will fail on > msysgit for similar reasons. Looking at test 6, the expected result is a > line ending with \r\n in text.txt. This line is extracted with grep > (grep 'justice and holiness' text.txt > actual), with unavoidable result > that on Cygwin the line ending is \n. This happens because on Cygwin, > the text utils are compiled to open files in text mode meaning than \n > and \r\n are both recognized as EOL markers. Thus, even though text.txt > is an exact match for what is created on Linux, the test fails because > \r\n cannot be distinguished by the available tools. > > I'm not sure the right way forward. I did confirm that by substituting > "q_to_tab" for "q_to_cr" in t3032, the test pass on Cygwin and on Linux. > Perhaps t3032 should be so amended to avoid use of a non-portable line > ending construct? This passes for me, on both cygwin and MinGW. After adding a test_pause to test #6: $ ./t3032-merge-recursive-options.sh -v ... expecting success: q_to_cr <<-\EOF >expected && justice and holiness and is the nurse of his age and theQ EOF git read-tree --reset -u HEAD && git merge-recursive --ignore-space-change HEAD^ -- HEAD remote && grep "justice and holiness" text.txt >actual && test_cmp expected actual && test_pause Merging HEAD with remote Merging: 0ab7224 Clarify be82dcf Remove cruft found 1 common ancestor: c1e95d9 Initial revision Auto-merging text.txt $ xxd expected 000: 2020 2020 6a75 7374 6963 6520 616e 6420 justice and 010: 686f 6c69 6e65 7373 2061 6e64 2069 7320 holiness and is 020: 7468 6520 6e75 7273 6520 6f66 2068 6973 the nurse of his 030: 2061 6765 2061 6e64 2074 6865 0d0aage and the.. $ xxd actual 000: 2020 2020 6a75 7374 6963 6520 616e 6420 justice and 010: 686f 6c69 6e65 7373 2061 6e64 2069 7320 holiness and is 020: 7468 6520 6e75 7273 6520 6f66 2068 6973 the nurse of his 030: 2061 6765 2061 6e64 2074 6865 0d0aage and the.. $ grep "justice and holiness" text.txt | xxd 000: 2020 2020 6a75 7374 6963 6520 616e 6420 justice and 010: 686f 6c69 6e65 7373 2061 6e64 2069 7320 holiness and is 020: 7468 6520 6e75 7273 6520 6f66 2068 6973 the nurse of his 030: 2061 6765 2061 6e64 2074 6865 0d0aage and the.. $ exit ... # passed all 11 test(s) 1..11 $ ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix some sparse warnings
Sparse issues some "Using plain integer as NULL pointer" warnings.
Each warning relates to the use of an '{0}' initialiser expression
in the declaration of an 'struct object_info'. The first field of
this structure has pointer type. Thus, in order to suppress these
warnings, we replace the initialiser expression with '{NULL}'.
Signed-off-by: Ramsay Jones
---
This was built on the next branch.
ATB,
Ramsay Jones
sha1_file.c | 2 +-
streaming.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 4c2365f..e4ab0a4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2440,7 +2440,7 @@ int sha1_object_info_extended(const unsigned char *sha1,
struct object_info *oi)
int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)
{
- struct object_info oi = {0};
+ struct object_info oi = {NULL};
oi.sizep = sizep;
return sha1_object_info_extended(sha1, &oi);
diff --git a/streaming.c b/streaming.c
index cac282f..5710065 100644
--- a/streaming.c
+++ b/streaming.c
@@ -135,7 +135,7 @@ struct git_istream *open_istream(const unsigned char *sha1,
struct stream_filter *filter)
{
struct git_istream *st;
- struct object_info oi = {0};
+ struct object_info oi = {NULL};
const unsigned char *real = lookup_replace_object(sha1);
enum input_source src = istream_source(real, type, &oi);
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t3032 - make compatible with systems using \r\n as a line ending
Jonathan Nieder wrote: > Mark Levedahl wrote: > >> Subtests 6, 7, and 9 rely test that merge-recursive correctly >> ignores whitespace when so directed. Change the particular whitespace >> sequences to be ones that are not known line endings so the whitespace >> is not changed when being extracted by line oriented grep. > > merge-recursive needs to be able to deal with \r at EOL, too, so if at > all possible I would prefer to see the test fixed to pass on Cygwin > some other way. Maybe use -U/--binary option to grep? Indeed, if you look at the top of that test file, you will see: test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b test_have_prereq MINGW && export GREP_OPTIONS=-U which may explain why it works for me on MinGW, but not why it works on cygwin 1.5. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix some sparse warnings
Jeff King wrote:
> On Tue, Jul 16, 2013 at 07:57:20AM +0200, Johannes Sixt wrote:
>
>> Am 7/15/2013 19:31, schrieb Ramsay Jones:
>>> Sparse issues three "Using plain integer as NULL pointer" warnings.
>>> Each warning relates to the use of an '{0}' initialiser expression
>>> in the declaration of an 'struct object_info'.
>>
>> I question the value of this warning. Initialization with '= {0}' is a
>> well-established idiom, and sparse should know about it. Also, plain 0
>> *is* a null pointer constant.
>
> I agree with you. It's not a bug, and I think sparse is being overly
> picky here; it is missing the forest for the trees in interpreting the
> idiom.
Yes, last time this came up, I looked at writing a patch to sparse to
allow this without complaint. It's still on my sparse TODO list, but
even if I finished it tonight, it would take a while to get into a
released version of sparse. ;-)
> Still, it may be worth tweaking in the name of eliminating compiler
> noise, since it does not cost us very much to do so (and I believe we
> have done so in the past, too).
>
> We could also ask people with sparse to turn off the "use NULL instead
> of 0" warning, but I think it _is_ a useful warning elsewhere (even
> though it is never a bug, it violates our style guidelines and may be an
> indication of a bug).
Indeed, if it wasn't for this, I would be happy to turn this warning off.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions
Mark Levedahl wrote:
> On 07/15/2013 10:06 PM, Torsten Bögershausen wrote:
>> On 2013-07-15 21.49, Junio C Hamano wrote:
>>> Mark Levedahl writes:
>>>
>>>>> In order to limit the adverse effects caused by this implementation,
>>>>> we provide a new "fast stat" interface, which allows us to use this
>>>>> only for interactions with the index (i.e. the cached stat data).
>>>>>
>>>>> Signed-off-by: Ramsay Jones
>>>>> ---
>>>> I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
>>>> using your prior patch (removing the Cygwin specific lstat entirely)
>>>> and get the same results with both, so this seems ok from me.
>>>>
>>>> My comparison point was created by reverting your current patch from
>>>> pu, then reapplying your earlier patch on top, so the only difference
>>>> was which approach was used to address the stat functions.
>>>>
>>>> Caveats:
>>>> 1) I don't find any speed improvement of the current patch over the
>>>> previous one (the tests actually ran faster with the earlier patch,
>>>> though the difference was less than 1%).
>> Hm, measuring the time for the test suite is one thing,
>> did you measure the time of "git status" with and without the patch?
>>
>> (I don't have my test system at hand, so I can test in a few days/weeks)
> Timing for 5 rounds of "git status" in the git project. First, with the
> current fast_lstat patches:
> /usr/local/src/git>for i in {1..5} ; do time git status >& /dev/null ; done
>
> real0m0.218s
> user0m0.000s
> sys 0m0.218s
>
> real0m0.187s
> user0m0.077s
> sys 0m0.109s
>
> real0m0.187s
> user0m0.030s
> sys 0m0.156s
>
> real0m0.203s
> user0m0.031s
> sys 0m0.171s
>
> real0m0.187s
> user0m0.062s
> sys 0m0.124s
>
> Now, with Ramsay's original patch just removing the non-Posix stat
> functions:
> /usr/local/src/git>for i in {1..5} ; do time git status >& /dev/null ; done
>
> real0m0.218s
> user0m0.046s
> sys 0m0.171s
>
> real0m0.187s
> user0m0.015s
> sys 0m0.171s
>
> real0m0.187s
> user0m0.015s
> sys 0m0.171s
>
> real0m0.187s
> user0m0.047s
> sys 0m0.140s
>
> real0m0.187s
> user0m0.031s
> sys 0m0.156s
>
>
> I see no difference in the above. (Yes, I checked multiple times that I
> was using different executables).
Hmm, that looks good. :-D
Torsten reported a performance boost using the win32 stat() implementation
on a linux git repo (2s -> 1s, if I recall correctly) on cygwin 1.7.
Do you have a larger repo available to test?
If performance isn't an issue (it isn't for _me_), then I will happily
re-submit my original patch (removing the win32 stat implementation).
[Hmm, I may do anyway!]
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Add the GIT_SENTINEL macro
The sentinel function attribute is not understood by versions of
the gcc compiler prior to v4.0. At present, for earlier versions
of gcc, the build issues 108 warnings related to the unknown
attribute. In order to suppress the warnings, we conditionally
define the GIT_SENTINEL macro to provide the sentinel attribute
for gcc v4.0 and newer.
Signed-off-by: Ramsay Jones
---
This was built on the next branch
ATB,
Ramsay Jones
argv-array.h | 2 +-
builtin/revert.c | 4 ++--
exec_cmd.h| 2 +-
git-compat-util.h | 7 +++
run-command.h | 2 +-
5 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/argv-array.h b/argv-array.h
index e805748..9a4c053 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -15,7 +15,7 @@ void argv_array_init(struct argv_array *);
void argv_array_push(struct argv_array *, const char *);
__attribute__((format (printf,2,3)))
void argv_array_pushf(struct argv_array *, const char *fmt, ...);
-__attribute__((sentinel))
+GIT_SENTINEL(0)
void argv_array_pushl(struct argv_array *, ...);
void argv_array_pop(struct argv_array *);
void argv_array_clear(struct argv_array *);
diff --git a/builtin/revert.c b/builtin/revert.c
index b8b5174..219f354 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,7 +54,7 @@ static int option_parse_x(const struct option *opt,
return 0;
}
-__attribute__((sentinel))
+GIT_SENTINEL(0)
static void verify_opt_compatible(const char *me, const char *base_opt, ...)
{
const char *this_opt;
@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char
*base_opt, ...)
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
}
-__attribute__((sentinel))
+GIT_SENTINEL(0)
static void verify_opt_mutually_compatible(const char *me, ...)
{
const char *opt1, *opt2 = NULL;
diff --git a/exec_cmd.h b/exec_cmd.h
index 307b55c..fbbb44d 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -7,7 +7,7 @@ extern const char *git_exec_path(void);
extern void setup_path(void);
extern const char **prepare_git_cmd(const char **argv);
extern int execv_git_cmd(const char **argv); /* NULL terminated */
-__attribute__((sentinel))
+GIT_SENTINEL(0)
extern int execl_git_cmd(const char *cmd, ...);
extern const char *system_path(const char *path);
diff --git a/git-compat-util.h b/git-compat-util.h
index ff193f4..cac8157 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -303,6 +303,13 @@ extern char *gitbasename(char *);
#endif
#endif
+/* The sentinel attribute is valid from gcc version 4.0 */
+#if defined(__GNUC__) && (__GNUC__ >= 4)
+#define GIT_SENTINEL(n) __attribute__((sentinel(n)))
+#else
+#define GIT_SENTINEL(n)
+#endif
+
#include "compat/bswap.h"
#ifdef USE_WILDMATCH
diff --git a/run-command.h b/run-command.h
index 0a47679..2f2b453 100644
--- a/run-command.h
+++ b/run-command.h
@@ -46,7 +46,7 @@ int finish_command(struct child_process *);
int run_command(struct child_process *);
extern char *find_hook(const char *name);
-__attribute__((sentinel))
+GIT_SENTINEL(0)
extern int run_hook(const char *index_file, const char *name, ...);
#define RUN_COMMAND_NO_STDIN 1
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] Add the NO_SENTINEL build variable
Jonathan Nieder wrote: > Ramsay Jones wrote: > >> One of the three gcc compilers that I use does not understand the >> sentinel function attribute. (so, it spews 108 warning messages) > > Do you know what version of gcc introduced the sentinel attribute? > Would it make sense for the ifdef in git-compat-util.h to be keyed on > __GNUC__ and __GNUC_MINOR__ instead of a new makefile flag? > I have on old (v4.2.1) gcc repo on Linux and looking at ~/gcc-4.2.1/gcc/ChangeLog-2004 I can see that the sentinel attribute was added on 2004-09-04 by Kaveh R. Ghazi. Also, I find "bump version string to version 4.0.0" was on 2004-09-09 and "bump version string to version 3.5.0" was on 2004-01-16. Several of my system header files (on Linux) imply that the sentinel attribute is supported by __GNUC__ >= 4. (One of them, ansidecl.h, states that gcc 3.5 supports it but ...) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/19] Index-v5
Thomas Gummerer wrote:
> Hi,
>
> previous rounds (without api) are at $gmane/202752, $gmane/202923,
> $gmane/203088 and $gmane/203517, the previous round with api was at
> $gmane/229732. Thanks to Junio, Duy and Eric for their comments on
> the previous round.
If I remember correctly, the original version of this series had the
same problem as Michael's "Fix some reference-related races" series
(now in master). In particular, you had introduced an 'index_changed()'
function which does essentially the same job as 'stat_validity_check()'
in the new reference handling API. I seem to remember advising you
not to compare st_uid, st_gid and st_ino on __CYGWIN__.
I haven't had time to look at this version of your series yet, but it
may be worth taking a look at stat_validity_check(). (although that is
causing failures on cygwin at the moment! ;-)
Also, I can't recall if I mentioned it to you at the time, but your
index reading code was (unnecessarily) calling munmap() twice on the
same buffer (without an intervening mmap()). This causes problems for
systems that have the NO_MMAP build variable set. In particular, the
compat/mmap.c code will attempt to free() the allocated memory block
twice, with unpredictable results.
I wrote a patch to address this at the time (Hmm, seems to be built
on v1.8.1), but didn't submit it since your patch didn't progress. :-D
I have included the patch below.
ATB,
Ramsay Jones
-- >8 --
From: Ramsay Jones
Date: Sun, 9 Sep 2012 20:50:32 +0100
Subject: [PATCH] mmap.c: Keep log of mmap() blocks to avoid double-delete bug
When compiling with the NO_MMAP build variable set, the built-in
'git_mmap()' and 'git_munmap()' compatibility routines use simple
memory allocation and file I/O to emulate the required behaviour.
The current implementation is vulnerable to the "double-delete" bug
(where the pointer returned by malloc() is passed to free() two or
more times), should the mapped memory block address be passed to
munmap() multiple times.
In order to guard the implementation from such a calling sequence,
we keep a list of mmap-block descriptors, which we then consult to
determine the validity of the input pointer to munmap(). This then
allows 'git_munmap()' to return -1 on error, as required, with
errno set to EINVAL.
Using a list in the log of mmap-ed blocks, along with the resulting
linear search, means that the performance of the code is directly
proportional to the number of concurrently active memory mapped
file regions. The number of such regions is not expected to be
excessive.
Signed-off-by: Ramsay Jones
---
compat/mmap.c | 57 -
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/compat/mmap.c b/compat/mmap.c
index c9d46d1..400e034 100644
--- a/compat/mmap.c
+++ b/compat/mmap.c
@@ -1,14 +1,61 @@
#include "../git-compat-util.h"
+struct mmbd { /* memory mapped block descriptor */
+ struct mmbd *next; /* next in list */
+ void *start; /* pointer to memory mapped block */
+ size_t length; /* length of memory mapped block */
+};
+
+static struct mmbd *head; /* head of mmb descriptor list */
+
+
+static void add_desc(struct mmbd *desc, void *start, size_t length)
+{
+ desc->start = start;
+ desc->length = length;
+ desc->next = head;
+ head = desc;
+}
+
+static void free_desc(struct mmbd *desc)
+{
+ if (head == desc)
+ head = head->next;
+ else {
+ struct mmbd *d = head;
+ for (; d; d = d->next) {
+ if (d->next == desc) {
+ d->next = desc->next;
+ break;
+ }
+ }
+ }
+ free(desc);
+}
+
+static struct mmbd *find_desc(void *start)
+{
+ struct mmbd *d = head;
+ for (; d; d = d->next) {
+ if (d->start == start)
+ return d;
+ }
+ return NULL;
+}
+
void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t
offset)
{
size_t n = 0;
+ struct mmbd *desc = NULL;
if (start != NULL || !(flags & MAP_PRIVATE))
die("Invalid usage of mmap when built with NO_MMAP");
start = xmalloc(length);
- if (start == NULL) {
+ desc = xmalloc(sizeof(*desc));
+ if (!start || !desc) {
+ free(start);
+ free(desc);
errno = ENOMEM;
return MAP_FAILED;
}
@@ -25,18 +72,26 @@ void *git_mmap(void *start, size_t length, int prot, int
flags, int fd, off_t of
if (errno == EAGAIN || errno == EINTR)
continue;
free(start);
+ free
Re: [PATCH] Use compat/regex on Cygwin
Mark Levedahl wrote: > Cygwin's regex library does not pass git's tests, so don't use it. This > fixes failures in t4018 and t4034. Hmm, these tests have always passed for me on cygwin. So, this is presumably a regression in the new-lib regex library versions used by cygwin 1.5 and cygwin 1.7. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Cygwin has trustable filemode
Mark Levedahl wrote:
> The supported Cygwin distribution on supported Windows versions provides
> complete support for POSIX filemodes, so enable this by default. git as
> distributed by the Cygwin project is configured this way.
>
> This fixes one testsuite failure:
> t3300 test 17 (diff-index -M -p with mode change quotes funny filename)
Huh? How is it running that test? Does cygwin 1.7 somehow allow tabs in
filenames? For me, on cygwin 1.5, that test reports:
$ ./t3300-funny-names.sh
1..0 # SKIP Your filesystem does not allow tabs in filenames
$
> Historical notes: Earlier versions of Cygwin (version 1.5 and prior) had
> various methods for supporting posix file modes on different file systems,
> often using extended attributes, and this support was optional. Such
> versions of Cygwin are not available on any public mirror and are not
> supported by the Cygwin project. The currently available Cygwin supports
> POSIX file modes without exception - this is not an optional
> configuration. The support does depend upon the underlying file system
> (neither Linux nor Cygwin can set an execute bit on a FAT file system as
> FAT has no such support), but as this is no different than Linux, the
> default should not treat Cygwin differently than Linux.
The motivation for the original patch had more to do with "windows people"
using win32 text editors which set the executable bit inappropriately.
(see commit c869753e).
Since I use cygwin tools (vim), I don't have this problem. :-D
> Users who desire the non-POSIX mode of operation must explicitly set
> core.filemode=False, accepting non-interoperability with Linux.
>
> Signed-off-by: Mark Levedahl
> ---
> config.mak.uname | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/config.mak.uname b/config.mak.uname
> index 7ac541e..779d06a 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -163,7 +163,6 @@ ifeq ($(uname_O),Cygwin)
> NO_THREAD_SAFE_PREAD = YesPlease
> NEEDS_LIBICONV = YesPlease
> NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
> - NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
> NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
> # There are conflicting reports about this.
> # On some boxes NO_MMAP is needed, and not so elsewhere.
>
Should you revert commit c869753e ("Force core.filemode to false
on Cygwin.", 30-12-2006) instead?
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions
Mark Levedahl wrote:
> On 07/10/2013 04:23 PM, Ramsay Jones wrote:
>> Commit adbc0b6b ("cygwin: Use native Win32 API for stat", 30-09-2008)
>> added a Win32 specific implementation of the stat functions. In order
>> to handle absolute paths, cygwin mount points and symbolic links, this
>> implementation may fall back on the standard cygwin l/stat() functions.
>> Also, the choice of cygwin or Win32 functions is made lazily (by the
>> first call(s) to l/stat) based on the state of some config variables.
>>
>> Unfortunately, this "schizophrenic stat" implementation has been the
>> source of many problems ever since. For example, see commits 7faee6b8,
>> 79748439, 452993c2, 085479e7, b8a97333, 924aaf3e, 05bab3ea and 0117c2f0.
>>
>> In order to limit the adverse effects caused by this implementation,
>> we provide a new "fast stat" interface, which allows us to use this
>> only for interactions with the index (i.e. the cached stat data).
>>
>> Signed-off-by: Ramsay Jones
>> ---
>
> I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
> using your prior patch (removing the Cygwin specific lstat entirely) and
> get the same results with both, so this seems ok from me.
Thank you for testing this.
> My comparison point was created by reverting your current patch from pu,
> then reapplying your earlier patch on top, so the only difference was
> which approach was used to address the stat functions.
>
> Caveats:
> 1) I don't find any speed improvement of the current patch over the
> previous one (the tests actually ran faster with the earlier patch,
> though the difference was less than 1%).
> 2) I still question this whole approach, especially having this
> non-POSIX compliant mode be the default. Running in this mode breaks
> interoperability with Linux, but providing a Linux environment is the
> *primary* goal of Cygwin.
Yes, I agree. Since I _always_ disable the Win32 stat functions (by
setting core.filemode by hand - yes it's a little annoying), I don't
get any "benefit" from the added complexity.
However, I don't use git on cygwin with *large* repositories. git.git
is pretty much as large as it gets. So, the difference in performance
for me amounts to something like 0.120s -> 0.260s, which I can barely
notice.
Other people may not be so lucky ...
I would be happy if my original patch (removing the win32 stat funcs)
was applied, but others may not be. :-P
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] Add the NO_SENTINEL build variable
Signed-off-by: Ramsay Jones
---
Hi Jeff,
One of the three gcc compilers that I use does not understand the
sentinel function attribute. (so, it spews 108 warning messages)
Is this, or something like it, too ugly for you to squash into
your patch? :-D
ATB,
Ramsay Jones
Makefile | 6 ++
argv-array.h | 2 +-
builtin/revert.c | 4 ++--
exec_cmd.h| 2 +-
git-compat-util.h | 6 ++
run-command.h | 2 +-
6 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index 9c0da06..63b539c 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,9 @@ all::
# Define NO_NORETURN if using buggy versions of gcc 4.6+ and profile feedback,
# as the compiler can crash (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49299)
#
+# Define NO_SENTINEL if you have a compiler which does not understand the
+# sentinel function attribute.
+#
# Define USE_NSEC below if you want git to care about sub-second file mtimes
# and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and
# it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely
@@ -1232,6 +1235,9 @@ endif
ifdef NO_NORETURN
BASIC_CFLAGS += -DNO_NORETURN
endif
+ifdef NO_SENTINEL
+ BASIC_CFLAGS += -DNO_SENTINEL
+endif
ifdef NO_NSEC
BASIC_CFLAGS += -DNO_NSEC
endif
diff --git a/argv-array.h b/argv-array.h
index e805748..31bc492 100644
--- a/argv-array.h
+++ b/argv-array.h
@@ -15,7 +15,7 @@ void argv_array_init(struct argv_array *);
void argv_array_push(struct argv_array *, const char *);
__attribute__((format (printf,2,3)))
void argv_array_pushf(struct argv_array *, const char *fmt, ...);
-__attribute__((sentinel))
+SENTINEL(0)
void argv_array_pushl(struct argv_array *, ...);
void argv_array_pop(struct argv_array *);
void argv_array_clear(struct argv_array *);
diff --git a/builtin/revert.c b/builtin/revert.c
index b8b5174..6aedc18 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,7 +54,7 @@ static int option_parse_x(const struct option *opt,
return 0;
}
-__attribute__((sentinel))
+SENTINEL(0)
static void verify_opt_compatible(const char *me, const char *base_opt, ...)
{
const char *this_opt;
@@ -71,7 +71,7 @@ static void verify_opt_compatible(const char *me, const char
*base_opt, ...)
die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
}
-__attribute__((sentinel))
+SENTINEL(0)
static void verify_opt_mutually_compatible(const char *me, ...)
{
const char *opt1, *opt2 = NULL;
diff --git a/exec_cmd.h b/exec_cmd.h
index 307b55c..75c0a82 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -7,7 +7,7 @@ extern const char *git_exec_path(void);
extern void setup_path(void);
extern const char **prepare_git_cmd(const char **argv);
extern int execv_git_cmd(const char **argv); /* NULL terminated */
-__attribute__((sentinel))
+SENTINEL(0)
extern int execl_git_cmd(const char *cmd, ...);
extern const char *system_path(const char *path);
diff --git a/git-compat-util.h b/git-compat-util.h
index 9f1eaca..e846e01 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -300,6 +300,12 @@ extern char *gitbasename(char *);
#endif
#endif
+#if defined(__GNUC__) && !defined(NO_SENTINEL)
+#define SENTINEL(n) __attribute__((sentinel(n)))
+#else
+#define SENTINEL(n)
+#endif
+
#include "compat/bswap.h"
#ifdef USE_WILDMATCH
diff --git a/run-command.h b/run-command.h
index 0a47679..8e75671 100644
--- a/run-command.h
+++ b/run-command.h
@@ -46,7 +46,7 @@ int finish_command(struct child_process *);
int run_command(struct child_process *);
extern char *find_hook(const char *name);
-__attribute__((sentinel))
+SENTINEL(0)
extern int run_hook(const char *index_file, const char *name, ...);
#define RUN_COMMAND_NO_STDIN 1
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix some sparse warnings
Sparse issues three "Using plain integer as NULL pointer" warnings.
Each warning relates to the use of an '{0}' initialiser expression
in the declaration of an 'struct object_info'. The first field of
this structure has pointer type. Thus, in order to suppress these
warnings, we replace the initialiser expression with '{NULL}'.
Signed-off-by: Ramsay Jones
---
Hi Jeff,
If you need to re-roll the patches in your 'jk/in-pack-size-measurement'
branch, could you please squash this (or something like it) into the
patches equivalent to commit 7c07385d ("zero-initialize object_info structs",
07-07-2013) [sha1_file.c and streaming.c] and commit 778d263a ("cat-file: add
--batch-disk-sizes option", 07-07-2013) [builtin/cat-file.c].
Thanks!
ATB,
Ramsay Jones
builtin/cat-file.c | 2 +-
sha1_file.c| 2 +-
streaming.c| 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index bf12883..860576e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -135,7 +135,7 @@ static int batch_one_object(const char *obj_name, int
print_contents)
if (print_contents == BATCH)
contents = read_sha1_file(sha1, &type, &size);
else if (print_contents == BATCH_DISK_SIZES) {
- struct object_info oi = {0};
+ struct object_info oi = {NULL};
oi.disk_sizep = &size;
type = sha1_object_info_extended(sha1, &oi);
}
diff --git a/sha1_file.c b/sha1_file.c
index 4c2365f..e4ab0a4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2440,7 +2440,7 @@ int sha1_object_info_extended(const unsigned char *sha1,
struct object_info *oi)
int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)
{
- struct object_info oi = {0};
+ struct object_info oi = {NULL};
oi.sizep = sizep;
return sha1_object_info_extended(sha1, &oi);
diff --git a/streaming.c b/streaming.c
index cac282f..5710065 100644
--- a/streaming.c
+++ b/streaming.c
@@ -135,7 +135,7 @@ struct git_istream *open_istream(const unsigned char *sha1,
struct stream_filter *filter)
{
struct git_istream *st;
- struct object_info oi = {0};
+ struct object_info oi = {NULL};
const unsigned char *real = lookup_replace_object(sha1);
enum input_source src = istream_source(real, type, &oi);
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jul 2013, #03; Tue, 9)
Junio C Hamano wrote: [ ... ] > * rr/send-email-ssl-verify (2013-07-06) 6 commits > - SQUASH??? update to support SSL_ca_file as well as SSL_ca_path > - SQUASH??? send-email: cover both smtps and starttls cases > - fixup! send-email: squelch warning from Net::SMTP::SSL > - SQUASH??? send-email giving default value to ssl-cert-path with ||= > assignment > - send-email: introduce sendemail.smtpsslcertpath > - send-email: squelch warning from Net::SMTP::SSL > > The issue seems a lot deeper than the initial attempt and needs > somebody to sit down and sort out to get the version dependencies > and lazy loading right. This causes test failures for me, since send-email can't load the IO/Socket/SSL.pm module. (on Linux, cygwin and MinGW!) [ ... ] > -- > [Stalled] > > * rj/read-default-config-in-show-ref-pack-refs (2013-06-17) 3 commits > - ### DONTMERGE: needs better explanation on what config they need > - pack-refs.c: Add missing call to git_config() > - show-ref.c: Add missing call to git_config() > > The changes themselves are probably good, but it is unclear what > basic setting needs to be read for which exact operation. > > Waiting for clarification. > $gmane/228294 Sorry, still on my TODO list. (Having said that, I'm no longer sure that these patches do the right thing! ;-) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Torsten Bögershausen wrote: > On 30.06.13 19:28, Ramsay Jones wrote: [ ... ] >>> You have just described my second patch! :D >> Unfortunately, I have not had any time to work on the patch this weekend. >> However, despite the patch being a bit rough around the edges, I decided >> to send it out (see below) to get some early feedback. >> >> Note that it passes the t3210, t3211, t5500, t3200, t3301, t7606 and t1301 >> tests, but I have not run the full test suite. >> >> Comments welcome. >> >> ATB, >> Ramsay Jones >> >> -- >8 -- >> Subject: [PATCH] cygwin: Add fast_[l]stat() functions >> >> Signed-off-by: Ramsay Jones >> --- >> builtin/apply.c| 6 +++--- >> builtin/commit.c | 2 +- >> builtin/ls-files.c | 2 +- >> builtin/rm.c | 2 +- >> builtin/update-index.c | 2 +- >> check-racy.c | 2 +- >> compat/cygwin.c| 12 >> compat/cygwin.h| 10 +++--- >> diff-lib.c | 2 +- >> diff.c | 2 +- >> entry.c| 4 ++-- >> git-compat-util.h | 13 +++-- >> help.c | 5 + >> path.c | 9 + >> preload-index.c| 2 +- >> read-cache.c | 6 +++--- >> unpack-trees.c | 8 >> 17 files changed, 36 insertions(+), 53 deletions(-) >> [ ... ] > I managed to apply your patch on next, and run the test suite before and after > your patch. > The performance running "git status" on a linux kernel is the same as in > v1.8.3. > > Running test suite did not show new failures. > The following test cases had failed, and are fixed now: > < t1400-update-ref > < t3210-pack-refs > < t3211-peel-ref > < t3306-notes-prune > < t5304-prune > < t5404-tracking-branches > < t5500-fetch-pack > < t5505-remote > < t5514-fetch-multiple > < t5515-fetch-merge-logic > < t6030-bisect-porcelain > < t9300-fast-import > < t9903-bash-prompt > > In short: Thanks, This looks good to me. Thank you for testing this! It is much appreciated. I sent a v2 patch to the list last night. It passes the full test suite for me on cygwin 1.5, although there appears to be a slight performance problem on MinGW (perhaps!). :( ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions
Commit adbc0b6b ("cygwin: Use native Win32 API for stat", 30-09-2008)
added a Win32 specific implementation of the stat functions. In order
to handle absolute paths, cygwin mount points and symbolic links, this
implementation may fall back on the standard cygwin l/stat() functions.
Also, the choice of cygwin or Win32 functions is made lazily (by the
first call(s) to l/stat) based on the state of some config variables.
Unfortunately, this "schizophrenic stat" implementation has been the
source of many problems ever since. For example, see commits 7faee6b8,
79748439, 452993c2, 085479e7, b8a97333, 924aaf3e, 05bab3ea and 0117c2f0.
In order to limit the adverse effects caused by this implementation,
we provide a new "fast stat" interface, which allows us to use this
only for interactions with the index (i.e. the cached stat data).
Signed-off-by: Ramsay Jones
---
builtin/apply.c| 8
builtin/commit.c | 2 +-
builtin/ls-files.c | 2 +-
builtin/rm.c | 2 +-
builtin/update-index.c | 2 +-
check-racy.c | 2 +-
compat/cygwin.c| 48 ++--
compat/cygwin.h| 17 +
diff-lib.c | 2 +-
diff.c | 2 +-
entry.c| 6 +++---
git-compat-util.h | 27 +--
help.c | 5 +
path.c | 9 +
preload-index.c| 2 +-
read-cache.c | 6 +++---
unpack-trees.c | 8
17 files changed, 68 insertions(+), 82 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 0e9b631..f5046a6 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3091,7 +3091,7 @@ static int checkout_target(struct cache_entry *ce, struct
stat *st)
memset(&costate, 0, sizeof(costate));
costate.base_dir = "";
costate.refresh_cache = 1;
- if (checkout_entry(ce, &costate, NULL) || lstat(ce->name, st))
+ if (checkout_entry(ce, &costate, NULL) || fast_lstat(ce->name, st))
return error(_("cannot checkout %s"), ce->name);
return 0;
}
@@ -3253,7 +3253,7 @@ static int load_current(struct image *image, struct patch
*patch)
if (pos < 0)
return error(_("%s: does not exist in index"), name);
ce = active_cache[pos];
- if (lstat(name, &st)) {
+ if (fast_lstat(name, &st)) {
if (errno != ENOENT)
return error(_("%s: %s"), name, strerror(errno));
if (checkout_target(ce, &st))
@@ -3396,7 +3396,7 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
if (previous) {
st_mode = previous->new_mode;
} else if (!cached) {
- stat_ret = lstat(old_name, st);
+ stat_ret = fast_lstat(old_name, st);
if (stat_ret && errno != ENOENT)
return error(_("%s: %s"), old_name, strerror(errno));
}
@@ -3850,7 +3850,7 @@ static void add_index_file(const char *path, unsigned
mode, void *buf, unsigned
die(_("corrupt patch for subproject %s"), path);
} else {
if (!cached) {
- if (lstat(path, &st) < 0)
+ if (fast_lstat(path, &st) < 0)
die_errno(_("unable to stat newly created file
'%s'"),
path);
fill_stat_cache_info(ce, &st);
diff --git a/builtin/commit.c b/builtin/commit.c
index 6b693c1..1d208c6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -231,7 +231,7 @@ static void add_remove_files(struct string_list *list)
if (p->util)
continue;
- if (!lstat(p->string, &st)) {
+ if (!fast_lstat(p->string, &st)) {
if (add_to_cache(p->string, &st, 0))
die(_("updating files failed"));
} else
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 3a410c3..a066719 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -247,7 +247,7 @@ static void show_files(struct dir_struct *dir)
continue;
if (ce_skip_worktree(ce))
continue;
- err = lstat(ce->name, &st);
+ err = fast_lstat(ce->name, &st);
if (show_deleted && err)
show_ce_entry(tag_removed, ce);
if (show_modified && ce_modified(ce, &st, 0))
diff --git a/builtin/rm.c b/builtin/rm.c
index 06025a2.
[RFC/PATCH v2 0/1] cygwin: fast stat functions
If you are wondering, v1 of this patch was appended to an email that is part of the "cygwin: Remove the Win32 l/stat() functions" thread. Changes from v1: - add comment in git-compat-util.h to explain the use of the "fast" stat variants. - remove the fast_stat() function, along with the now redundant code in compat/cygwin.c - add the fast_fstat() function; this is used by code in write_entry() (entry.c:139), even though it is not actually called by cygwin. - replaced an additional call to lstat with fast_lstat. - a commit message This patch, which was built on master @ commit 56df44a, has passed the full test suite: $ GIT_SKIP_TESTS='t0008 t0061.3 t0070.3 t4130 t9010 t9300' \ make test >test-outp15 2>&1 $ After Torsten's patch, I didn't need to skip t0070.3; that was just force of habit! Once Mark's PIPE prerequisite patch is applied, I would not have to skip t0008, t9010 and t9300. I also used one of Torsten's tests, on git.git, to provide a quick check on performance: $ time bin-wrappers/git -c core.filemode=true status -uno which gave the following results (5 runs, discard fastest, slowest and average remaining three): master @ 56df44a + this patch true 0.641, 0.656, (0.688), 0.657, (0.640) = 0.651 false 0.500, (0.531), 0.500, (0.469), 0.500 = 0.500 master @ 56df44a true 0.670, (0.734), (0.638), 0.648, 0.663 = 0.660 false (0.509), (0.490), 0.496, 0.497, 0.498 = 0.497 So, this patch does not cause any performance regression (+/- 1%). However, I was a bit worried by the timings, until I noticed that (on cygwin) the timings are affected by running from the build directory. Note the timings for v1.8.3: v1.8.3 (build) true 0.671, 0.672, (0.640), 0.672, (0.969) = 0.672 false (0.515), 0.532, 0.516, 0.516, (0.547) = 0.521 v1.8.3 (installed) true (0.250), (0.266), 0.265, 0.250, 0.265 = 0.260 false (0.109), (0.125), 0.109, 0.125, 0.125 = 0.120 Also, just for comparison, here are the numbers for Linux and MinGW on the same laptop: Linux MinGW master + patch0.045 0.115 master @ 56df44a - 0.110 v1.8.3 (build)0.045 0.109 v1.8.3 (installed)0.045 0.094 So, this patch seems to cause a 5% slowdown on MinGW; I haven't looked into this yet. Note that the "build directory slowdown" is much less pronounced on MinGW, and non existent (or too small to notice) on Linux. This patch is marked RFC because: - I don't like the function names; suggestions welcome! - Is fast_fstat() necessary; should we remove the use of fstat() in write_entry() instead. (I don't think so). - Is the 5% slowdown on MinGW a real problem? (are the static inline functions being in-lined?) - I need to double check that I have replaced all relevant lstat() calls. ATB, Ramsay Jones Ramsay Jones (1): cygwin: Add fast_lstat() and fast_fstat() functions builtin/apply.c| 8 builtin/commit.c | 2 +- builtin/ls-files.c | 2 +- builtin/rm.c | 2 +- builtin/update-index.c | 2 +- check-racy.c | 2 +- compat/cygwin.c| 48 ++-- compat/cygwin.h| 17 + diff-lib.c | 2 +- diff.c | 2 +- entry.c| 6 +++--- git-compat-util.h | 27 +-- help.c | 5 + path.c | 9 + preload-index.c| 2 +- read-cache.c | 6 +++--- unpack-trees.c | 8 17 files changed, 68 insertions(+), 82 deletions(-) -- 1.8.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
free(new);
if (wrote != size)
@@ -209,8 +193,7 @@ static int write_entry(struct cache_entry *ce, char
*path, const struct checkout
finish:
if (state->refresh_cache) {
- if (!fstat_done)
- fast_lstat(ce->name, &st);
+ fast_lstat(ce->name, &st);
fill_stat_cache_info(ce, &st);
}
return 0;
--
I would need to do some timing tests (on Linux) to see what effect this
would have on performance. (I noticed that the test suite ran about 2%
slower with the above applied).
A simple pass-though fast_fstat(), including on cygwin, is probably the
way to go.
Sorry for being a bit slow on this - I'm very busy at the moment. :(
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] show-ref.c: Add missing call to git_config()
Junio C Hamano wrote: > Ramsay Jones writes: > >> Yes, I will send a v2 (soon-ish, I hope). > > Ping? > > No need to hurry, but just to make sure this didn't disappear from > everybody's radar. Yep, this is still on my TODO list. Sorry for being tardy on these patches. :( ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Ramsay Jones wrote:
> Michael Haggerty wrote:
>> On 06/27/2013 12:35 AM, Jeff King wrote:
> [ ... ]
>>> I think Michael's assessment above is missing one thing.
>>
>> Peff is absolutely right; for some unknown reason I was thinking of the
>> consistency check as having been already fixed.
>
> Well, the "cygwin: Remove the Win32 l/stat() functions" patch *does* fix
> the problem. :-D It's just a pity we can't use it on performance grounds. :(
>
>>> [...#ifdef out consistency check on cygwin when lock is held...]
>>
>> Yes, this would work.
>>
>> But, taking a step back, I think it is a bad idea to have an unreliable
>> stat() masquerading as a real stat(). If we want to allow the use of an
>> unreliable stat for certain purposes, let's have two stat() interfaces:
>>
>> * the true stat() (in this case I guess cygwin's slow-but-correct
>> implementation)
>>
>> * some fast_but_maybe_unreliable_stat(), which would map to stat() on
>> most platforms but might map to the Windows stat() on cygwin when so
>> configured.
>>
>> By default the true stat() would always be used. It should have to be a
>> conscious decision, taken only in specific, vetted scenarios, to use the
>> unreliable stat.
>
> You have just described my second patch! :D
Unfortunately, I have not had any time to work on the patch this weekend.
However, despite the patch being a bit rough around the edges, I decided
to send it out (see below) to get some early feedback.
Note that it passes the t3210, t3211, t5500, t3200, t3301, t7606 and t1301
tests, but I have not run the full test suite.
Comments welcome.
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] cygwin: Add fast_[l]stat() functions
Signed-off-by: Ramsay Jones
---
builtin/apply.c| 6 +++---
builtin/commit.c | 2 +-
builtin/ls-files.c | 2 +-
builtin/rm.c | 2 +-
builtin/update-index.c | 2 +-
check-racy.c | 2 +-
compat/cygwin.c| 12
compat/cygwin.h| 10 +++---
diff-lib.c | 2 +-
diff.c | 2 +-
entry.c| 4 ++--
git-compat-util.h | 13 +++--
help.c | 5 +
path.c | 9 +
preload-index.c| 2 +-
read-cache.c | 6 +++---
unpack-trees.c | 8
17 files changed, 36 insertions(+), 53 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 0e9b631..ca26caa 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3253,7 +3253,7 @@ static int load_current(struct image *image, struct patch
*patch)
if (pos < 0)
return error(_("%s: does not exist in index"), name);
ce = active_cache[pos];
- if (lstat(name, &st)) {
+ if (fast_lstat(name, &st)) {
if (errno != ENOENT)
return error(_("%s: %s"), name, strerror(errno));
if (checkout_target(ce, &st))
@@ -3396,7 +3396,7 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
if (previous) {
st_mode = previous->new_mode;
} else if (!cached) {
- stat_ret = lstat(old_name, st);
+ stat_ret = fast_lstat(old_name, st);
if (stat_ret && errno != ENOENT)
return error(_("%s: %s"), old_name, strerror(errno));
}
@@ -3850,7 +3850,7 @@ static void add_index_file(const char *path, unsigned
mode, void *buf, unsigned
die(_("corrupt patch for subproject %s"), path);
} else {
if (!cached) {
- if (lstat(path, &st) < 0)
+ if (fast_lstat(path, &st) < 0)
die_errno(_("unable to stat newly created file
'%s'"),
path);
fill_stat_cache_info(ce, &st);
diff --git a/builtin/commit.c b/builtin/commit.c
index 6b693c1..1d208c6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -231,7 +231,7 @@ static void add_remove_files(struct string_list *list)
if (p->util)
continue;
- if (!lstat(p->string, &st)) {
+ if (!fast_lstat(p->string, &st)) {
if (add_to_cache(p->string, &st, 0))
die(_("updating files failed"));
} else
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 08d9786..db66a0e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -251,7 +251,7 @@ static void show_files(struct dir_struct *dir)
contin
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Michael Haggerty wrote: > On 06/27/2013 12:35 AM, Jeff King wrote: [ ... ] >> I think Michael's assessment above is missing one thing. > > Peff is absolutely right; for some unknown reason I was thinking of the > consistency check as having been already fixed. Well, the "cygwin: Remove the Win32 l/stat() functions" patch *does* fix the problem. :-D It's just a pity we can't use it on performance grounds. :( >> [...#ifdef out consistency check on cygwin when lock is held...] > > Yes, this would work. > > But, taking a step back, I think it is a bad idea to have an unreliable > stat() masquerading as a real stat(). If we want to allow the use of an > unreliable stat for certain purposes, let's have two stat() interfaces: > > * the true stat() (in this case I guess cygwin's slow-but-correct > implementation) > > * some fast_but_maybe_unreliable_stat(), which would map to stat() on > most platforms but might map to the Windows stat() on cygwin when so > configured. > > By default the true stat() would always be used. It should have to be a > conscious decision, taken only in specific, vetted scenarios, to use the > unreliable stat. You have just described my second patch! :D > > For example, I can't imagine that checking the freshness of the index or > of the packed-refs file is ever going to be a bottleneck, so there is no > reason at all to use an unreliable stat() here. > > On the other hand, stat() seems definitely to be a bottleneck when > testing for changes in a 100,000 file working tree, and here occasional > mistakes might be considered acceptable. So for this purpose the > unreliable stat() might be used. I have already written the first pass at this patch, but I'm having difficulty with naming (get_cached_stat_data, get_index_stat_data, get_stat_data, ... ;-) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Torsten Bögershausen wrote:
[ ... ]
>>> (And have a look how to improve the core.filemode)
>>
>> I don't understand this (parenthetical) comment; could you
>> elaborate on this.
>>
> This is probably wrong information:
> I had in mind that cygwin sets core.filemode=false,
It does, see commit c869753e ("Force core.filemode to false on
Cygwin", 30-12-2006).
> which is quite annoying when exchanging .sh files with linux.
Indeed, I used to build git with NO_TRUSTABLE_FILEMODE reset so
that I wouldn't have to edit the config file by hand after a
git-clone or git-init.
> But that seems to be wrong, a quick test shows that core.filemode=true.
Hmm, it shouldn't - confused!
> Sorry for confusion.
ATB
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Jeff King wrote:
> On Wed, Jun 26, 2013 at 06:35:52PM -0400, Jeff King wrote:
>
>> I am curious how often Cygwin gives us the false positive. If it is
>> every time, then the check is not doing much good at all. Is it possible
>> for you to instrument stat_validity_check to report how often it does or
>> does not do anything useful?
>
> Maybe like this:
>
> diff --git a/read-cache.c b/read-cache.c
> index d5201f9..19dcb69 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1958,6 +1958,14 @@ void stat_validity_clear(struct stat_validity *sv)
> sv->sd = NULL;
> }
>
> +static int unchanged;
> +static int attempts;
> +static void print_stats(void)
> +{
> + fprintf(stderr, "stat data was unchanged %d/%d\n",
> + unchanged, attempts);
> +}
> +
> int stat_validity_check(struct stat_validity *sv, const char *path)
> {
> struct stat st;
> @@ -1966,7 +1974,16 @@ int stat_validity_check(struct stat_validity *sv,
> const char *path)
> return sv->sd == NULL;
> if (!sv->sd)
> return 0;
> - return S_ISREG(st.st_mode) && !match_stat_data(sv->sd, &st);
> + if (!S_ISREG(st.st_mode))
> + return 0;
> + if (!attempts++)
> + atexit(print_stats);
> + if (!match_stat_data(sv->sd, &st)) {
> + unchanged++;
> + return 1;
> + }
> + else
> + return 0;
> }
>
> void stat_validity_update(struct stat_validity *sv, int fd)
>
> Running "t3211 -v", I see things like:
>
> stat data was unchanged 3/3
> stat data was unchanged 20/20
> stat data was unchanged 2/2
> Deleted branch yadda (was d1ff1c9).
> stat data was unchanged 8/8
> ok 8 - peeled refs survive deletion of packed ref
>
> I am curious if you will see 0/20 or 19/20 there on Cygwin.
Ah, no ... the problem is not as subtle as this! :-D
I'm sorry if I failed to mention that I already had a patch
to solve the problem, but that I *don't* want to submit it,
since it is ugly and just serves to spread the madness! ;-)
This is why I tried the "cygwin: Remove the Win32 l/stat()
functions" patch first; I think this is the correct approach
to fixing this problem (and similar *future* problems).
However, since that is no longer an option, on performance
grounds, I have other ideas I want to try. (see later email)
The "schizophrenic stat" functions have caused many subtle
problems, but this is not one of them; a brick to the head
would be more subtle. The problem is caused by the "stat
validity" code using both fstat() and stat() on the same
file. Note that there is no Win32 implementation of the
fstat() function.
So, a solution to the problem would be to provide just such
an implementation. The patch to do so is given below, just
for illustration purposes.
This fixes the failures to t3210, t3211 and t5500. However, I
have not run the full test suite. This may cause some further
*subtle* problems (just grep for fstat and consider what may
go wrong; I have not done that), especially when you consider
that cygwin has the UNRELIABLE_FSTAT build variable set.
HTH
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] cygwin: Add an Win32 fstat implementation
Signed-off-by: Ramsay Jones
---
compat/cygwin.c | 41 +
compat/cygwin.h | 3 +++
2 files changed, 44 insertions(+)
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 91ce5d4..d59c9fb 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -4,6 +4,7 @@
#include
#include "win32.h"
#include "../git-compat-util.h"
+#include
#include "../cache.h" /* to read configuration */
/*
@@ -100,6 +101,38 @@ static int cygwin_stat(const char *path, struct stat *buf)
return do_stat(path, buf, stat);
}
+static int cygwin_fstat(int fd, struct stat *buf)
+{
+ HANDLE fh = (HANDLE)get_osfhandle(fd);
+ BY_HANDLE_FILE_INFORMATION fdata;
+
+ if (fh == INVALID_HANDLE_VALUE) {
+ errno = EBADF;
+ return -1;
+ }
+ if (GetFileType(fh) != FILE_TYPE_DISK)
+ return (fstat)(fd, buf);
+ if (GetFileInformationByHandle(fh, &fdata)) {
+ buf->st_dev = buf->st_rdev = 0; /* not used by Git */
+ buf->st_ino = 0;
+ buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes);
+ buf->st_nlink = 1;
+ buf->st_uid = buf->st_gid = 0;
+#ifdef __CYGWIN_USE_BIG_TYPES__
+ buf->st_size = ((_off64_t)fdata.nFileSizeHigh << 32) +
+ fdata.nFileSizeLow;
+#else
+ buf->st_size = (off_t)fdata.nFileSizeLow;
+#endif
+ buf-
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Jeff King wrote: > On Wed, Jun 26, 2013 at 10:45:48PM +0100, Ramsay Jones wrote: [ ... ] > I think Michael's assessment above is missing one thing. It is true that > a false positive is just a performance problem in most cases, as we > unnecessarily reload the file, thinking it has changed. > > However, when we have taken a lock on the file, there is an additional > safety measure: if we find the file is changed, we abort, as that should > never happen (it means somebody else modified the file while we had it > locked). But of course Cygwin's false positive here triggers the safety > valve, and we die without even realizing that nothing changed. Indeed, this is exactly the situation. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Michael Haggerty wrote: > On 06/25/2013 07:07 AM, Junio C Hamano wrote: >> Ramsay Jones writes: >> >>> Michael Haggerty and Jeff King have been re-vamping the reference >>> handling code. The failures noted above were provoked by patches >>> in the 'mh/ref-races' branch. At the time I wrote this patch, that >>> branch was only included in 'pu', but I notice that this topic has >>> now progressed to 'next' (see commit 71f1a182). >> >> I had an impression that up to 98eeb09e (for_each_ref: load all >> loose refs before packed refs, 2013-06-20) that is now in 'next' >> does not agressively use the lstat timestamp of the packed-refs >> file, and the "optional" bit 5d478f5c (refs: do not invalidate the >> packed-refs cache unnecessarily, 2013-06-20), and the one in 'next' >> should be safe with the cheating-lstat. Isn't it the case? >> >> In any case, if removing the cheating-lstat will improve the >> robustness without hurting performance, I am all for it. >> >> The less platform specific hacks, the better ;-). > > The following patch in the "non-optional" commits (which has already > been merged to next) uses stat() via the new stat_validity API: > > ca9199300e get_packed_ref_cache: reload packed-refs file when it changes > > This patch adds some *extra* cache invalidation that was heretofore > missing. If stat() is broken it could > > (a) cause a false positive, resulting in some unnecessary cache > invalidation and re-reading of packed-refs, which will hurt performance > but not correctness; or > > (b) cause a false negative, in which case the stale cache might be used > for reading (but not writing), just as was *always* the case before this > patch. > > As far as I understand, the concern for cygwin is (a). I will leave it > to others to measure and/or decide whether the performance loss is too > grave to endure until the cygwin stat() situation is fixed. > Hmm, I'm not sure I understand ... However, I can confirm that the 'mh/ref-races' branch in next is broken on cygwin. (i.e. it is not just a speed issue; it provokes fatal errors). See reply to Junio. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Junio C Hamano wrote:
> Ramsay Jones writes:
>
>> Michael Haggerty and Jeff King have been re-vamping the reference
>> handling code. The failures noted above were provoked by patches
>> in the 'mh/ref-races' branch. At the time I wrote this patch, that
>> branch was only included in 'pu', but I notice that this topic has
>> now progressed to 'next' (see commit 71f1a182).
>
> I had an impression that up to 98eeb09e (for_each_ref: load all
> loose refs before packed refs, 2013-06-20) that is now in 'next'
> does not agressively use the lstat timestamp of the packed-refs
> file, and the "optional" bit 5d478f5c (refs: do not invalidate the
> packed-refs cache unnecessarily, 2013-06-20), and the one in 'next'
> should be safe with the cheating-lstat. Isn't it the case?
The next branch (from a couple of days ago, namely commit 4f488db) is
currently broken on cygwin, like so:
$ cd t
$ ./t3211-peel-ref.sh -i -v
[ ... ]
ok 7 - refs are peeled outside of refs/tags (old packed)
expecting success:
git pack-refs --all &&
cp .git/packed-refs fully-peeled &&
git branch yadda &&
git pack-refs --all &&
git branch -d yadda &&
test_cmp fully-peeled .git/packed-refs
fatal: internal error: packed-ref cache cleared while locked
not ok 8 - peeled refs survive deletion of packed ref
#
# git pack-refs --all &&
# cp .git/packed-refs fully-peeled &&
# git branch yadda &&
# git pack-refs --all &&
# git branch -d yadda &&
# test_cmp fully-peeled .git/packed-refs
#
$ cd trash\ directory.t3211-peel-ref/
$ ls
actual base.t expect
$ ../../bin-wrappers/git pack-refs --all
fatal: internal error: packed-ref cache cleared while locked
$ gdb ../../git.exe
GNU gdb 6.5.50.20060706-cvs (cygwin-special)
Copyright (C) 2006 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for details.
This GDB was configured as "i686-pc-cygwin"...
(gdb) b stat_validity_check
Breakpoint 1 at 0x48a33f: file read-cache.c, line 1965.
(gdb) run pack-refs --all
Starting program: /home/ramsay/git/git.exe pack-refs --all
Loaded symbols for /cygdrive/c/WINDOWS/system32/ntdll.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/kernel32.dll
Loaded symbols for /usr/bin/cygcrypto-0.9.8.dll
Loaded symbols for /usr/bin/cygwin1.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/advapi32.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/rpcrt4.dll
Loaded symbols for /cygdrive/c/WINDOWS/system32/secur32.dll
Loaded symbols for /usr/bin/cygiconv-2.dll
Loaded symbols for /usr/bin/cygz.dll
Breakpoint 1, stat_validity_check (sv=0xa611a4,
path=0x57d98c ".git/packed-refs") at read-cache.c:1965
1965if (stat(path, &st) < 0)
(gdb) n
1967if (!sv->sd)
(gdb) p sv
$1 = (struct stat_validity *) 0xa611a4
(gdb) p *sv
$2 = {sd = 0xa62478}
(gdb) p sv->sd
$3 = (struct stat_data *) 0xa62478
(gdb) p *sv->sd
$4 = {sd_ctime = {sec = 1372194879, nsec = 671875000}, sd_mtime = {
sec = 1372194879, nsec = 65625}, sd_dev = 2899104371,
sd_ino = 180184, sd_uid = 1005, sd_gid = 513, sd_size = 296}
(gdb) p st
$5 = {st_dev = 0, st_ino = 0, st_mode = 33152, st_nlink = 1, st_uid = 0,
st_gid = 0, st_rdev = 0, st_size = 296, st_atim = {tv_sec = 1372195048,
tv_nsec = 890625000}, st_mtim = {tv_sec = 1372194879,
tv_nsec = 65625}, st_ctim = {tv_sec = 1372194879,
tv_nsec = 15625000}, st_blksize = 5636381, st_blocks = 1, st_spare4 = {
10883480, 5757324}}
(gdb) c
Continuing.
fatal: internal error: packed-ref cache cleared while locked
Program exited with code 0200.
(gdb) quit
$ ../../test-stat .git/packed-refs
stat for '.git/packed-refs':
*dev: -1395862925, 0
*ino: 180184, 0
*mode: 100644 -rw-, 100600 -rw-
nlink: 1, 1
*uid: 1005, 0
*gid: 513, 0
*rdev: -1395862925, 0
size: 296, 296
atime: 1372195048, 1372195048 Tue Jun 25 22:17:28 2013
mtime: 1372194879, 1372194879 Tue Jun 25 22:14:39 2013
ctime: 1372194879, 1372194879 Tue Jun 25 22:14:39 2013
$
Note that, as noted before, (at least) the following tests fail
due to the 'mh/ref-races' branch:
t3210-pack-refs.sh(Wstat: 256 Tests: 16 Failed: 12)
Failed tests: 4-9, 11-16
Non-zero exit status: 1
t3211-peel-ref.sh
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Torsten Bögershausen wrote: > On 2013-06-25 23.18, Junio C Hamano wrote: >> Johannes Sixt writes: >> >>> Some context: This is about a patch by Ramsay that removes the >>> "schizophrenic lstat" hack for Cygwin. Junio, can you please queue that >>> patch in pu? >> >> Sure. Thanks. > > First of all, > thanks for the work. > > Here some "benchmark" results, > (The test run of the test suite did the same amout of time). The test suite runs noticeably faster for me. > > But: > git status -uno in real life takes double the time, > git 1.8.3 compared against "pu with the vanilla l/stat" > > 1 second -> 2 seconds on linux kernel > 0.2 seconds -> 0.4 seconds on git.git Hmm, OK, I guess I will have to try something else. Sigh :( > Do we have any known problems with the current implementation ? Yes. The next branch is currently broken. (see reply to Junio) > Does speed matter ? > > One vote to keep the special cygwin functions. > (And have a look how to improve the core.filemode) I don't understand this (parenthetical) comment; could you elaborate on this. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] show-ref.c: Add missing call to git_config()
Junio C Hamano wrote: > Ramsay Jones writes: > >> At present, 'git show-ref' ignores any attempt to set config >> variables (e.g. core.checkstat) from the command line using >> the -c option to git. > > I think what you really want to see is not giving "-c" and have it > honored. > > "git show-ref" does not honor configuration variables at > all, but at least core configuration variables read by > git_default_config (most importantly core.checkstat) should > be read and honored, because ... > > would be more appropriate. What are the variables that matter to > show-ref, and what are the reasons why they should be honored? I > thought show-ref was just a way to enumerate refs, and does not read > the index nor checks if there are modifications in the working tree, > so I do not see any reason offhand for it to honor core.checkstat > (and I think that is the reason why we don't have the call there in > the current code). :-D Yes, you caught me! These patches *may* not be necessary, prior to Michael's "reference related races" series. Specifically, the introduction of the stat_validity_check() function to the reference handling API. This means that the behaviour 'git show-ref' is now affected by several config variables, including core.checkstat. I haven't spent any time auditing the code, but the list would include (at least) core.trustctime, core.filemode, core.checkstat, core.ignorecygwinfstricks, ... > > Exactly the same comment applies to 2/2. ditto > Note that I am _not_ opposing these changes. You brought them up > because you saw some reason why these should honor some core > variables. I just want that reason to be explained in the log for > the future developers. Yes, I will send a v2 (soon-ish, I hope). ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/12] Fix some reference-related races
Michael Haggerty wrote: > Thanks for all of the information. > > On 06/15/2013 10:13 PM, Ramsay Jones wrote: >> Michael Haggerty wrote: >>> *This patch series must be built on top of mh/reflife.* [ ... ] >> You may be wondering why clear_packed_ref_cache() is called? Well, that >> is because stat_validity_check() *incorrectly* indicates that the >> packed-refs file has changed. Why does it do that? Well, this is one >> more example of the problems caused by the cygwin schizophrenic stat() >> functions. :( [ARGH] >> [ ... ] > So if I understand correctly, all of the above is *without* the > refcounting changes introduced in mh/ref-races? Is so, then it is not > surprising, as this is exactly the sort of problem that the reference > counting is meant to solve. Yes, as I said, this describes the old (non refcounted) series. This particular problem (the segmentation fault) is fixed by the new series (as noted below). [Note, however, that the packed-refs file will still be re-read more often than needed.] >> Now, turning to the new code, t3211-peel-ref.sh test #7 now works, but >> test #8 still fails... [ ... ] > These "internal error: packed-ref cache cleared while locked" failures > result from an internal consistency check that clear_packed_ref_cache() > is not called while the write lock is held on the packed-refs file. A > call to c_p_r_c() could result from > > * a programming error > > * a determination based on the packed-refs file stat that the file needs > to be re-read > > Judging from what you said about cygwin, I assume that the latter is > happening. Indeed. > It should be impossible, because the current process is > holding packed-refs.lock, and therefore other git processes should > refuse to change the packed-refs file. :-P You are assuming that a single process can't lie to itself ... [ ... ] > Yikes! ECYGWINFAIL. Ah, NO, this should read ECYGWINGITFAIL. This is a self-inflicted wound; it has nothing much to do with cygwin. I should not have assumed that you knew what I meant by "schizophrenic stat() functions" above; sorry about that! If you are interested, then the following commits may be useful reading: adbc0b6, 7faee6b, 7974843, 05bab3ea, 924aaf3e and b8a97333. [ ... ] >> I haven't checked the remaining test failures to see if they are >> caused by this code (I don't think so, but ...), but this failure >> is clearly a cygwin specific issue. > > Thanks again for the testing and analysis, So, unless you feel the need to fix this yourself, you can probably ignore this issue for now. I will hopefully find time to fix it up before this topic progresses to next. (Although I don't have any feeling for the time-frame of this topic). HTH ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Add missing calls to git_config()
Hi Junio,
I recently had need to use the '-c' option to git in order to
set some config variables from the command line; specifically
with 'git show-ref' and 'git pack-refs'. I haven't looked to
see if any other commands need similar attention, but:
$ git grep 'cmd_.*(int argc' -- builtin | wc -l
109
$ git grep 'git_config(' -- builtin | wc -l
80
$
... maybe. ;-)
[I did think about separating the command line processing from
the config file processing; maybe some commands could accept
config settings from the command line, but do not need/want
the regular config file processing? *dunno*.]
ATB,
Ramsay Jones
Ramsay Jones (2):
show-ref.c: Add missing call to git_config()
pack-refs.c: Add missing call to git_config()
builtin/pack-refs.c | 3 +++
builtin/show-ref.c | 2 ++
2 files changed, 5 insertions(+)
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/12] Fix some reference-related races
ive deletion of packed ref # # git pack-refs --all && # cp .git/packed-refs fully-peeled && # git branch yadda && # git pack-refs --all && # git branch -d yadda && # test_cmp fully-peeled .git/packed-refs # $ cd trash\ directory.t3211-peel-ref/ $ ../../bin-wrappers/git pack-refs --all fatal: internal error: packed-ref cache cleared while locked $ ls actual base.t expect $ ls .git COMMIT_EDITMSG branches/ description index logs/ packed-refs HEADconfig hooks-disabled/ info/ objects/ refs/ $ ls -l .git/packed-refs -rw-r--r-- 1 ramsay None 296 Jun 14 20:34 .git/packed-refs $ cat .git/packed-refs # pack-refs with: peeled d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master eb0e854c2cd2511c7571b1a5e3c8b8146193fb30 refs/outside/foo d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/tags/base eb0e854c2cd2511c7571b1a5e3c8b8146193fb30 refs/tags/foo ^d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e $ Now, I have a test-stat program which prints the difference between the two stat implementations used in cygwin git, thus: $ ../../test-stat .git/packed-refs stat for '.git/packed-refs': *dev: -1395862925, 0 *ino: 166044, 0 *mode: 100644 -rw-, 100600 -rw- nlink: 1, 1 *uid: 1005, 0 *gid: 513, 0 *rdev: -1395862925, 0 size: 296, 296 atime: 1371238550, 1371238550 Fri Jun 14 20:35:50 2013 mtime: 1371238469, 1371238469 Fri Jun 14 20:34:29 2013 ctime: 1371238469, 1371238469 Fri Jun 14 20:34:29 2013 $ ../../bin-wrappers/git -c core.checkstat=minimal pack-refs --all fatal: internal error: packed-ref cache cleared while locked $ Hmmm, that should have worked! Wait, fix 'git pack-refs' to support setting config variables on the command line, rebuild and: $ ../../bin-wrappers/git -c core.checkstat=minimal pack-refs --all $ cat .git/packed-refs # pack-refs with: peeled fully-peeled d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master eb0e854c2cd2511c7571b1a5e3c8b8146193fb30 refs/outside/foo ^d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/tags/base eb0e854c2cd2511c7571b1a5e3c8b8146193fb30 refs/tags/foo ^d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e $ I haven't checked the remaining test failures to see if they are caused by this code (I don't think so, but ...), but this failure is clearly a cygwin specific issue. HTH ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] show-ref.c: Add missing call to git_config()
At present, 'git show-ref' ignores any attempt to set config variables (e.g. core.checkstat) from the command line using the -c option to git. In order to enable such usage, add the missing call to git_config() from cmd_show_ref(). Signed-off-by: Ramsay Jones --- builtin/show-ref.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 8d9b76a..95f5ca9 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -191,6 +191,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix) if (argc == 2 && !strcmp(argv[1], "-h")) usage_with_options(show_ref_usage, show_ref_options); + git_config(git_default_config, NULL); + argc = parse_options(argc, argv, prefix, show_ref_options, show_ref_usage, PARSE_OPT_NO_INTERNAL_HELP); -- 1.8.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] pack-refs.c: Add missing call to git_config()
At present, 'git pack-refs' ignores any attempt to set config
variables (e.g. core.checkstat) from the command line using
the -c option to git. In order to enable such usage, add the
missing call to git_config() from cmd_pack_refs().
Signed-off-by: Ramsay Jones
---
builtin/pack-refs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index b20b1ec..ade1ae5 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -15,6 +15,9 @@ int cmd_pack_refs(int argc, const char **argv, const char
*prefix)
OPT_BIT(0, "prune", &flags, N_("prune loose refs (default)"),
PACK_REFS_PRUNE),
OPT_END(),
};
+
+ git_config(git_default_config, NULL);
+
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
return pack_refs(flags);
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] object.c: Fix a sparse warning
Sparse issues an "'object_array_slopbuf' not declared. Should it be
static?" warning. In order to suppress the warning, since this
symbol does not need more than file visibility, we simply add the
static modifier to its declaration.
Signed-off-by: Ramsay Jones
---
Hi Michael,
If you need to re-roll the patches in your 'mh/reflife' branch,
could you please squash this into the patch corresponding to
commit cbdeb23e ("object_array_entry: fix memory handling of
the name field", 25-05-2013).
Thanks!
ATB,
Ramsay Jones
object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/object.c b/object.c
index 2976b2e..0de4c91 100644
--- a/object.c
+++ b/object.c
@@ -269,7 +269,7 @@ int object_list_contains(struct object_list *list, struct
object *obj)
* A zero-length string to which object_array_entry::name can be
* initialized without requiring a malloc/free.
*/
-char object_array_slopbuf[1];
+static char object_array_slopbuf[1];
static void add_object_array_with_mode_context(struct object *obj, const char
*name,
struct object_array *array,
--
1.8.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND v2] path: Fix a sparse warning
Torsten Bögershausen wrote: > On 2013-05-28 19.04, Junio C Hamano wrote: >> >> Can you tell me what the conclusion on the discussion on your two >> other patches on 'pu'? >> >> * rj/mingw-cygwin (2013-05-08) 2 commits >> - cygwin: Remove the CYGWIN_V15_WIN32API build variable >> - mingw: rename WIN32 cpp macro to GIT_WINDOWS_NATIVE >> >> I stopped keeping track of the discussion and my vague recollection >> was that it is OK for 1.5 but not verified on 1.7 or something? >> > > The tests I did under cygwin 1.7 did not show any regression. Thank you for testing this. ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (May 2013, #08; Tue, 28)
Junio C Hamano wrote: > > * rj/mingw-compat-st-mode-bits (2013-05-28) 1 commit > - path: Fix a sparse warning > > Will merge to 'next'. Hmm, this breaks the build on cygwin. :( Now, I always test my patches on Linux, cygwin and MinGW before sending, ... except that I obviously didn't test on cygwin this time - it doesn't even compile. *ahem* The extern declaration for get_st_mode_bits() must come *before* including compat/cygwin.h; otherwise it will get mangled by the macro in that header file. Sorry for breaking the build. A v3 patch is on it's way. (I'm liking the v1 patch more now ;-) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] path: Fix a sparse warning
On MinGW, sparse issues an "'get_st_mode_bits' not declared. Should it be static?" warning. The MinGW and MSVC builds do not see the declaration of this function, within git-compat-util.h, due to its placement within an preprocessor conditional. In order to suppress the warning, we simply move the declaration to the top level of the header. Signed-off-by: Ramsay Jones --- Hi Junio, As promised, this fixes the build on cygwin. (tested on cygwin and MinGW. I have not tested on Linux, because I would have to re-boot twice, and I wanted to send this out tonight). Again, sorry for breaking the build. ATB, Ramsay Jones git-compat-util.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index 660b7f0..aa0404e 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -127,6 +127,9 @@ #else #include #endif + +extern int get_st_mode_bits(const char *path, int *mode); + #if defined(__MINGW32__) /* pull in Windows compatibility stuff */ #include "compat/mingw.h" @@ -163,7 +166,6 @@ typedef long intptr_t; typedef unsigned long uintptr_t; #endif -int get_st_mode_bits(const char *path, int *mode); #if defined(__CYGWIN__) #undef _XOPEN_SOURCE #include -- 1.8.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RESEND v2] path: Fix a sparse warning
On MinGW, sparse issues an "'get_st_mode_bits' not declared. Should it be static?" warning. The MinGW and MSVC builds do not see the declaration of this function, within git-compat-util.h, due to its placement within an preprocessor conditional. In order to suppress the warning, we simply move the declaration to the top level of the header. Signed-off-by: Ramsay Jones --- Hi Junio, Now that v1.8.3 is out, I note that this patch seems to have been dropped (or did I miss something?). This used to be [PATCH 2/6] path: Make the 'get_st_mode_bits' symbol a file static but the change in implementation required a change in title. This version simply moves the declaration so that the MinGW and MSVC builds can see it. ATB, Ramsay Jones git-compat-util.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/git-compat-util.h b/git-compat-util.h index e955bb5..0e5e4f8 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -163,7 +163,6 @@ typedef long intptr_t; typedef unsigned long uintptr_t; #endif -int get_st_mode_bits(const char *path, int *mode); #if defined(__CYGWIN__) #undef _XOPEN_SOURCE #include @@ -176,6 +175,8 @@ int get_st_mode_bits(const char *path, int *mode); #endif #endif +extern int get_st_mode_bits(const char *path, int *mode); + /* used on Mac OS X */ #ifdef PRECOMPOSE_UNICODE #include "compat/precompose_utf8.h" -- 1.8.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
