* Re: [PATCH] clean.c, ls-files.c: respect encapsulation of exclude_list_groups
From: Junio C Hamano @ 2013-01-16 22:20 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1358342758-30503-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> Consumers of the dir.c traversal API should avoid assuming knowledge
> of the internal implementation of exclude_list_groups. Therefore
> when adding items to an exclude list, it should be accessed via the
> pointer returned from add_exclude_list(), rather than by referencing
> a location within dir.exclude_list_groups[EXC_CMDL].
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
> builtin/clean.c | 6 +++---
> builtin/ls-files.c | 15 ++++++++++-----
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/clean.c b/builtin/clean.c
> index b098288..b9cb7ad 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -45,6 +45,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> static const char **pathspec;
> struct strbuf buf = STRBUF_INIT;
> struct string_list exclude_list = STRING_LIST_INIT_NODUP;
> + struct exclude_list *el;
When a type "exclude_list" exists and used in the same function,
having a local variable of the same name but of a different type
becomes a bit awkward.
builtin/ls-files.c shares the same structure. Does the file-scope
"exclude_args" variable need to be a file-scope static over there?
It seems that it is closely tied to the elements of the string list,
so it may make sense to:
* remove the file-scope static "exclude_args";
* rename "exclude_list" string list variable to "exclude_args";
and
* replace "--exclude_args" in the loop that iterates over
exclude_list (now exclude_args) with "-(i+1)" or something,
just like you do in "builtin/clean.c" below.
> - add_exclude_list(&dir, EXC_CMDL, "--exclude option");
> + el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
> for (i = 0; i < exclude_list.nr; i++)
> - add_exclude(exclude_list.items[i].string, "", 0,
> - &dir.exclude_list_group[EXC_CMDL].el[0], -(i+1));
> + add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
We may want to use for_each_string_list_item() here and in the
corresponding loop in builtin/ls-files.c, but because we do need to
give the -(i + 1) label to each element, I think the code is OK
as-is.
^ permalink raw reply
* [PATCH 2/2] fix clang -Wtautological-compare with unsigned enum
From: Antoine Pelisse @ 2013-01-16 22:47 UTC (permalink / raw)
To: John Keeping, Max Horn, Junio C Hamano
Cc: git, Johannes Sixt, Antoine Pelisse
In-Reply-To: <1358376443-7404-1-git-send-email-apelisse@gmail.com>
Create a GREP_HEADER_FIELD_MIN so we can check that the field value is
sane and silent the clang warning.
Clang warning happens because the enum is unsigned (this is
implementation-defined, and there is no negative fields) and the check
is then tautological.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
I tried to consider discussion [1] and this [2] discussion on clang's list
With these two patches and the patch from Max Horne, I'm finally able to
compile with CC=clang CFLAGS=-Werror.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/184908
[2]: http://clang-developers.42468.n3.nabble.com/Possibly-invalid-enum-tautology-warning-td3233140.html
grep.c | 3 ++-
grep.h | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/grep.c b/grep.c
index 4bd1b8b..bb548ca 100644
--- a/grep.c
+++ b/grep.c
@@ -625,7 +625,8 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
for (p = opt->header_list; p; p = p->next) {
if (p->token != GREP_PATTERN_HEAD)
die("bug: a non-header pattern in grep header list.");
- if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
+ if (p->field < GREP_HEADER_FIELD_MIN ||
+ GREP_HEADER_FIELD_MAX <= p->field)
die("bug: unknown header field %d", p->field);
compile_regexp(p, opt);
}
diff --git a/grep.h b/grep.h
index 8fc854f..e4a1df5 100644
--- a/grep.h
+++ b/grep.h
@@ -28,7 +28,8 @@ enum grep_context {
};
enum grep_header_field {
- GREP_HEADER_AUTHOR = 0,
+ GREP_HEADER_FIELD_MIN = 0,
+ GREP_HEADER_AUTHOR = GREP_HEADER_FIELD_MIN,
GREP_HEADER_COMMITTER,
GREP_HEADER_REFLOG,
--
1.8.1.1.435.g20d29be.dirty
^ permalink raw reply related
* [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
From: Antoine Pelisse @ 2013-01-16 22:47 UTC (permalink / raw)
To: John Keeping, Max Horn, Junio C Hamano
Cc: git, Johannes Sixt, Antoine Pelisse
In-Reply-To: <20130116182449.GA4881@sigill.intra.peff.net>
clang incorrectly reports a constant conversion warning (implicit
truncation to bit field) when using the "flag &= ~FLAG" form, because
~FLAG needs to be truncated.
Convert this form to "flag = flag & ~FLAG" fixes the issue as
the right operand now fits into the bit field.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
I'm sorry about this fix, it really seems bad, yet it's one step closer
to warning-free clang compilation.
It seems quite clear to me that it's a bug in clang.
bisect.c | 2 +-
builtin/checkout.c | 2 +-
builtin/reflog.c | 4 ++--
commit.c | 4 ++--
revision.c | 8 ++++----
upload-pack.c | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/bisect.c b/bisect.c
index bd1b7b5..34ac01d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -63,7 +63,7 @@ static void clear_distance(struct commit_list *list)
{
while (list) {
struct commit *commit = list->item;
- commit->object.flags &= ~COUNTED;
+ commit->object.flags = commit->object.flags & ~COUNTED;
list = list->next;
}
}
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a9c1b5a..2c83234 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -717,7 +717,7 @@ static void orphaned_commit_warning(struct commit *old, struct commit *new)
init_revisions(&revs, NULL);
setup_revisions(0, NULL, &revs, NULL);
- object->flags &= ~UNINTERESTING;
+ object->flags = object->flags & ~UNINTERESTING;
add_pending_object(&revs, object, sha1_to_hex(object->sha1));
for_each_ref(add_pending_uninteresting_ref, &revs);
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..3079c81 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -170,7 +170,7 @@ static int commit_is_complete(struct commit *commit)
}
/* clear flags from the objects we traversed */
for (i = 0; i < found.nr; i++)
- found.objects[i].item->flags &= ~STUDYING;
+ found.objects[i].item->flags = found.objects[i].item->flags& ~STUDYING;
if (is_incomplete)
commit->object.flags |= INCOMPLETE;
else {
@@ -229,7 +229,7 @@ static void mark_reachable(struct expire_reflog_cb *cb)
struct commit_list *leftover = NULL;
for (pending = cb->mark_list; pending; pending = pending->next)
- pending->item->object.flags &= ~REACHABLE;
+ pending->item->object.flags = pending->item->object.flags & ~REACHABLE;
pending = cb->mark_list;
while (pending) {
diff --git a/commit.c b/commit.c
index e8eb0ae..800779d 100644
--- a/commit.c
+++ b/commit.c
@@ -883,7 +883,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
/* Uniquify */
for (p = heads; p; p = p->next)
- p->item->object.flags &= ~STALE;
+ p->item->object.flags = p->item->object.flags & ~STALE;
for (p = heads, num_head = 0; p; p = p->next) {
if (p->item->object.flags & STALE)
continue;
@@ -894,7 +894,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)
for (p = heads, i = 0; p; p = p->next) {
if (p->item->object.flags & STALE) {
array[i++] = p->item;
- p->item->object.flags &= ~STALE;
+ p->item->object.flags = p->item->object.flags & ~STALE;
}
}
num_head = remove_redundant(array, num_head);
diff --git a/revision.c b/revision.c
index d7562ee..ed1c16d 100644
--- a/revision.c
+++ b/revision.c
@@ -787,9 +787,9 @@ static void limit_to_ancestry(struct commit_list *bottom, struct commit_list *li
/* We are done with the TMP_MARK */
for (p = list; p; p = p->next)
- p->item->object.flags &= ~TMP_MARK;
+ p->item->object.flags = p->item->object.flags & ~TMP_MARK;
for (p = bottom; p; p = p->next)
- p->item->object.flags &= ~TMP_MARK;
+ p->item->object.flags = p->item->object.flags & ~TMP_MARK;
free_commit_list(rlist);
}
@@ -1948,7 +1948,7 @@ static int remove_duplicate_parents(struct commit *commit)
/* count them while clearing the temporary mark */
surviving_parents = 0;
for (p = commit->parents; p; p = p->next) {
- p->item->object.flags &= ~TMP_MARK;
+ p->item->object.flags = p->item->object.flags & ~TMP_MARK;
surviving_parents++;
}
return surviving_parents;
@@ -2378,7 +2378,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
if (revs->reflog_info) {
fake_reflog_parent(revs->reflog_info, commit);
- commit->object.flags &= ~(ADDED | SEEN | SHOWN);
+ commit->object.flags = commit->object.flags & ~(ADDED | SEEN | SHOWN);
}
/*
diff --git a/upload-pack.c b/upload-pack.c
index 7c05b15..74d8f0e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -113,7 +113,7 @@ static int do_rev_list(int in, int out, void *user_data)
for (i = 0; i < want_obj.nr; i++) {
struct object *o = want_obj.objects[i].item;
/* why??? */
- o->flags &= ~UNINTERESTING;
+ o->flags = o->flags & ~UNINTERESTING;
add_pending_object(&revs, o, NULL);
}
for (i = 0; i < have_obj.nr; i++) {
@@ -700,7 +700,7 @@ static void receive_needs(void)
struct commit_list *parents;
packet_write(1, "unshallow %s",
sha1_to_hex(object->sha1));
- object->flags &= ~CLIENT_SHALLOW;
+ object->flags = object->flags & ~CLIENT_SHALLOW;
/* make sure the real parents are parsed */
unregister_shallow(object->sha1);
object->parsed = 0;
--
1.8.1.1.435.g20d29be.dirty
^ permalink raw reply related
* Re: Question re. git remote repository
From: Stephen Smith @ 2013-01-16 22:59 UTC (permalink / raw)
To: Konstantin Khomoutov
Cc: Jeff King, Konstantin Khomoutov, git@vger.kernel.org, Lang, David
In-Reply-To: <20130116233744.7d0775eaec98ce154a9de180@domain007.com>
>>>> Ideally we'd prefer to simply create our remote repository on a
>>>> drive of one of our local network servers. Is this possible?
>>>
>>> Yes, this is possible, but it's not advised to keep such a
>>> "reference" repository on an exported networked drive for a number
>>> of reasons (both performance and bug-free operation).
>>
>> I agree that performance is not ideal (although if you are on a fast
>> LAN, it probably would not matter much), but I do not recall any
>> specific bugs in that area. Can you elaborate?
>
> This one [1] for instance. I also recall seing people having other
> "mystical" problems with setups like this so I somehow developed an idea
> than having a repository on a networked drive is asking for troubles.
> Of course, if there are happy users of such setups, I would be glad to
> hear as my precautions might well be unfounded for the recent versions
> of Git.
>
> 1. http://code.google.com/p/msysgit/issues/detail?id=130
A group I was with used a master repository on a windows share for quite some time without a database corruption being seen.
^ permalink raw reply
* Re: Question re. git remote repository
From: David Lang @ 2013-01-16 23:00 UTC (permalink / raw)
To: Stephen Smith
Cc: Konstantin Khomoutov, Jeff King, git@vger.kernel.org, Lang, David
In-Reply-To: <0630A778-9AC8-4023-889C-4FC58ABAB683@gmail.com>
On Wed, 16 Jan 2013, Stephen Smith wrote:
>>>>> Ideally we'd prefer to simply create our remote repository on a
>>>>> drive of one of our local network servers. Is this possible?
>>>>
>>>> Yes, this is possible, but it's not advised to keep such a
>>>> "reference" repository on an exported networked drive for a number
>>>> of reasons (both performance and bug-free operation).
>>>
>>> I agree that performance is not ideal (although if you are on a fast
>>> LAN, it probably would not matter much), but I do not recall any
>>> specific bugs in that area. Can you elaborate?
>>
>> This one [1] for instance. I also recall seing people having other
>> "mystical" problems with setups like this so I somehow developed an idea
>> than having a repository on a networked drive is asking for troubles.
>> Of course, if there are happy users of such setups, I would be glad to
>> hear as my precautions might well be unfounded for the recent versions
>> of Git.
>>
>> 1. http://code.google.com/p/msysgit/issues/detail?id=130
>
> A group I was with used a master repository on a windows share for quite some time without a database corruption being seen. --
I think the risk is that if you have multiple people doing actions on the shared
filesystem you can run into trouble.
As long as only one copy of git is ever running against the repository, I don't
see any reason for there to be a problem.
But if you try to have one filesystem, with multiple people running git on their
machines against that shared filesystem, I would expect you to have all sorts of
problems.
David Lang
^ permalink raw reply
* Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
From: John Keeping @ 2013-01-16 23:08 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: Max Horn, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <1358376443-7404-1-git-send-email-apelisse@gmail.com>
On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote:
> clang incorrectly reports a constant conversion warning (implicit
> truncation to bit field) when using the "flag &= ~FLAG" form, because
> ~FLAG needs to be truncated.
>
> Convert this form to "flag = flag & ~FLAG" fixes the issue as
> the right operand now fits into the bit field.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
> I'm sorry about this fix, it really seems bad, yet it's one step closer
> to warning-free clang compilation.
>
> It seems quite clear to me that it's a bug in clang.
Which version of clang did you see this with? I don't get these
warnings with clang 3.2.
John
^ permalink raw reply
* Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
From: Antoine Pelisse @ 2013-01-16 23:09 UTC (permalink / raw)
To: John Keeping; +Cc: Max Horn, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <20130116230800.GB4574@serenity.lan>
On Thu, Jan 17, 2013 at 12:08 AM, John Keeping <john@keeping.me.uk> wrote:
> On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote:
>> clang incorrectly reports a constant conversion warning (implicit
>> truncation to bit field) when using the "flag &= ~FLAG" form, because
>> ~FLAG needs to be truncated.
> Which version of clang did you see this with? I don't get these
> warnings with clang 3.2.
Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0)
It's good to know it's been fixed !
^ permalink raw reply
* Re: [PATCH 2/2] fix clang -Wtautological-compare with unsigned enum
From: Antoine Pelisse @ 2013-01-16 23:10 UTC (permalink / raw)
To: John Keeping, Max Horn, Junio C Hamano
Cc: git, Johannes Sixt, Antoine Pelisse
In-Reply-To: <1358376443-7404-2-git-send-email-apelisse@gmail.com>
> With these two patches and the patch from Max Horne,
I'm deeply sorry for this typo Max
^ permalink raw reply
* Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
From: Antoine Pelisse @ 2013-01-16 23:15 UTC (permalink / raw)
To: John Keeping; +Cc: Max Horn, Junio C Hamano, git, Johannes Sixt
In-Reply-To: <CALWbr2ypYUvuE4pWfcVvVcnJkRvCNrM1gVHp_UXeke9gbgoE3A@mail.gmail.com>
So I guess we should drop this patch, it's probably not worth it,
especially if it's been fixed already by clang.
On Thu, Jan 17, 2013 at 12:09 AM, Antoine Pelisse <apelisse@gmail.com> wrote:
> On Thu, Jan 17, 2013 at 12:08 AM, John Keeping <john@keeping.me.uk> wrote:
>> On Wed, Jan 16, 2013 at 11:47:22PM +0100, Antoine Pelisse wrote:
>>> clang incorrectly reports a constant conversion warning (implicit
>>> truncation to bit field) when using the "flag &= ~FLAG" form, because
>>> ~FLAG needs to be truncated.
>> Which version of clang did you see this with? I don't get these
>> warnings with clang 3.2.
>
> Ubuntu clang version 3.0-6ubuntu3 (tags/RELEASE_30/final) (based on LLVM 3.0)
>
> It's good to know it's been fixed !
^ permalink raw reply
* Re: Question re. git remote repository
From: Matt Seitz (matseitz) @ 2013-01-16 23:32 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: ishchis2@gmail.com, david@lang.hm
"David Lang" <david@lang.hm> wrote in message news:<alpine.DEB.2.02.1301161459060.21503@nftneq.ynat.uz>...
> But if you try to have one filesystem, with multiple people running git on their
> machines against that shared filesystem, I would expect you to have all sorts of
> problems.
What leads you to think you will have problems?
Why would there be more of a problem on a network file system as opposed to local file system that can be accessed by multiple users?
Linus seemed to think it should work:
http://permalink.gmane.org/gmane.comp.version-control.git/122670
And "git init" specifically has a "shared" option:
--shared[=(false|true|umask|group|all|world|everybody|0xxx)]
Specify that the git repository is to be shared amongst several users. This allows users belonging to the same group to push into that repository. When specified, the config variable "core.sharedRepository" is set so that files and directories under $GIT_DIR are created with the requested permissions. When not specified, git will use permissions reported by umask(2).
^ permalink raw reply
* Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
From: Junio C Hamano @ 2013-01-16 23:43 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: John Keeping, Max Horn, git, Johannes Sixt
In-Reply-To: <1358376443-7404-1-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> clang incorrectly reports a constant conversion warning (implicit
> truncation to bit field) when using the "flag &= ~FLAG" form, because
> ~FLAG needs to be truncated.
>
> Convert this form to "flag = flag & ~FLAG" fixes the issue as
> the right operand now fits into the bit field.
If the "clang incorrectly reports" is already recognised by clang
folks as a bug to be fixed in clang, I'd rather not to take this
patch.
I do not think it is reasonable to expect people to remember that
they have to write "flags &= ~TO_DROP" in a longhand whenever they
are adding new code that needs to do bit-fields, so even if this
patch makes clang silent for the _current_ code, it will not stay
that way. Something like
#define FLIP_BIT_CLR(fld,bit) do { \
typeof(fld) *x = &(fld); \
*x = *x & (~(bit)); \
} while (0)
may be more palapable but not by a large margin.
Yuck.
^ permalink raw reply
* Re: [PATCH 1/2] fix clang -Wconstant-conversion with bit fields
From: Junio C Hamano @ 2013-01-16 23:46 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: John Keeping, Max Horn, git, Johannes Sixt
In-Reply-To: <7vpq14vglu.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Antoine Pelisse <apelisse@gmail.com> writes:
>
>> clang incorrectly reports a constant conversion warning (implicit
>> truncation to bit field) when using the "flag &= ~FLAG" form, because
>> ~FLAG needs to be truncated.
>>
>> Convert this form to "flag = flag & ~FLAG" fixes the issue as
>> the right operand now fits into the bit field.
>
> If the "clang incorrectly reports" is already recognised by clang
> folks as a bug to be fixed in clang, I'd rather not to take this
> patch.
>
> I do not think it is reasonable to expect people to remember that
> they have to write "flags &= ~TO_DROP" in a longhand whenever they
> are adding new code that needs to do bit-fields, so even if this
> patch makes clang silent for the _current_ code, it will not stay
> that way. Something like
>
> #define FLIP_BIT_CLR(fld,bit) do { \
> typeof(fld) *x = &(fld); \
> *x = *x & (~(bit)); \
> } while (0)
>
> may be more palapable but not by a large margin.
>
> Yuck.
Double yuck. I meant palatable.
In any case, I see somebody reports that more recent clang does not
have this bug in the near-by message, so let's forget about this
issue.
Thanks.
^ permalink raw reply
* Re: Question re. git remote repository
From: David Lang @ 2013-01-17 0:21 UTC (permalink / raw)
To: Matt Seitz (matseitz); +Cc: git@vger.kernel.org, ishchis2@gmail.com
In-Reply-To: <A0DB01D693D8EF439496BC8B037A0AEF322098A4@xmb-rcd-x15.cisco.com>
On Wed, 16 Jan 2013, Matt Seitz (matseitz) wrote:
> "David Lang" <david@lang.hm> wrote in message news:<alpine.DEB.2.02.1301161459060.21503@nftneq.ynat.uz>...
>> But if you try to have one filesystem, with multiple people running git on their
>> machines against that shared filesystem, I would expect you to have all sorts of
>> problems.
>
> What leads you to think you will have problems?
>
> Why would there be more of a problem on a network file system as opposed to
> local file system that can be accessed by multiple users?
There are safety checks and synchronization primitives that work between
mulitiple users on one machine (where you can see what other processes are
running for example) that don't work with separate machines using a filesystem
> Linus seemed to think it should work:
>
> http://permalink.gmane.org/gmane.comp.version-control.git/122670
well, he knows git better than I do, but using git over NFS/CIFS is not the same
as saying that you have multiple users on different systems making changes.
In the link you point at, he says that you can have problems with some types of
actions. He points out things like git prune, but I would also say that there
are probably race conditions if you have two git processes that try to change
the HEAD to different things at the same time.
> And "git init" specifically has a "shared" option:
>
> --shared[=(false|true|umask|group|all|world|everybody|0xxx)]
>
> Specify that the git repository is to be shared amongst several users. This
> allows users belonging to the same group to push into that repository. When
> specified, the config variable "core.sharedRepository" is set so that files
> and directories under $GIT_DIR are created with the requested permissions.
> When not specified, git will use permissions reported by umask(2).
>
I think this is dealing with multiple users _reading_ a repository, not making
updates to it at the same time.
David Lang
^ permalink raw reply
* Re: [PATCH 3/8] git_remote_helpers: Force rebuild if python version changes
From: Pete Wyckoff @ 2013-01-17 0:27 UTC (permalink / raw)
To: John Keeping; +Cc: git, Eric S. Raymond, Felipe Contreras, Sverre Rabbelier
In-Reply-To: <20130115225805.GA4574@serenity.lan>
john@keeping.me.uk wrote on Tue, 15 Jan 2013 22:58 +0000:
> For reference, putting the version check in setup.py looks like this:
>
> -- >8 --
>
> diff --git a/git_remote_helpers/setup.py b/git_remote_helpers/setup.py
> index 6de41de..2c21eb5 100644
> --- a/git_remote_helpers/setup.py
> +++ b/git_remote_helpers/setup.py
> @@ -3,6 +3,7 @@
> """Distutils build/install script for the git_remote_helpers package."""
>
> from distutils.core import setup
> +import sys
>
> # If building under Python3 we need to run 2to3 on the code, do this by
> # trying to import distutils' 2to3 builder, which is only available in
> @@ -13,6 +14,24 @@ except ImportError:
> # 2.x
> from distutils.command.build_py import build_py
>
> +
> +current_version = '%d.%d' % sys.version_info[:2]
> +try:
> + f = open('GIT-PYTHON_VERSION', 'r')
> + latest_version = f.read().strip()
> + f.close()
> +
> + if latest_version != current_version:
> + if not '--force' in sys.argv:
> + sys.argv.insert(0, '--force')
> +except IOError:
> + pass
> +
> +f = open('GIT-PYTHON_VERSION', 'w')
> +f.write(current_version)
> +f.close()
> +
> +
> setup(
> name = 'git_remote_helpers',
> version = '0.1.0',
>
That's about the same overhead as doing it in the Makefile,
and a bit more obscure. I don't mind your initial version
so much anymore. Thanks for thinking about it.
-- Pete
^ permalink raw reply
* Re: [RFC/PATCH 2/8 v3] git_remote_helpers: fix input when running under Python 3
From: Pete Wyckoff @ 2013-01-17 0:29 UTC (permalink / raw)
To: John Keeping
Cc: Junio C Hamano, Michael Haggerty, git, Eric S. Raymond,
Felipe Contreras, Sverre Rabbelier
In-Reply-To: <20130116094418.GA9089@river>
john@keeping.me.uk wrote on Wed, 16 Jan 2013 09:45 +0000:
> On Tue, Jan 15, 2013 at 07:03:16PM -0500, Pete Wyckoff wrote:
> > I'd suggest for this Python conundrum using byte-string literals, e.g.:
> >
> > lines = check_output(args).strip().split(b'\n')
> > value, name = line.split(b' ')
> > name = name.strip(b'commit\t')
> >
> > Essentially identical to what you have, but avoids naming "utf-8" as
> > the encoding. It instead relies on Python's interpretation of
> > ASCII characters in string context, which is exactly what C does.
>
> The problem is that AFAICT the byte-string prefix is only available in
> Python 2.7 and later (compare [1] and [2]). I think we need this more
> convoluted code if we want to keep supporting Python 2.6 (although
> perhaps 'ascii' would be a better choice than 'utf-8').
>
> [1] http://docs.python.org/2.6/reference/lexical_analysis.html#literals
> [2] http://docs.python.org/2.7/reference/lexical_analysis.html#literals
Drat. The b'' syntax seems to work on 2.6.8, in spite of
the docs, but certainly isn't in 2.5.
I think you had hit on the best compromise with encoding,
but maybe ascii is a little less presumptuous than utf-8,
and more indicative of the encoding assumption.
-- Pete
^ permalink raw reply
* Re: [PATCH] Add Auto-Submitted header to post-receive-email
From: Jonathan Nieder @ 2013-01-17 0:49 UTC (permalink / raw)
To: Chris Hiestand
Cc: Andy Parkins, Junio C Hamano, git, Michael Haggerty,
Kevin P. Fleming
In-Reply-To: <258F0FE2-D014-4624-A1E2-721E51F0E12C@salk.edu>
Hi Chris,
Chris Hiestand wrote:
> Andy, do you have any thoughts on adding this email header to
> contrib/hooks/post-receive-email? This patch shouldn't cause problems for anyone
> with a sanely configured mail delivery agent, and the additional header is very
> useful in toggling auto responses.
I'm not Andy, but it sounds very sane to me. So for what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks,
Jonathan
(patch left unsnipped for reference)
> This conforms to RFC3834 and is useful in preventing eg
> vacation auto-responders from replying by default
>
> Signed-off-by: Chris Hiestand <chiestand@salk.edu>
> ---
> contrib/hooks/post-receive-email | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
> index b2171a0..0e5b72d 100755
> --- a/contrib/hooks/post-receive-email
> +++ b/contrib/hooks/post-receive-email
> @@ -237,6 +237,7 @@ generate_email_header()
> X-Git-Reftype: $refname_type
> X-Git-Oldrev: $oldrev
> X-Git-Newrev: $newrev
> + Auto-Submitted: auto-generated
>
> This is an automated email from the git hooks/post-receive script. It was
> generated because a ref change was pushed to the repository containing
> --
> 1.7.10.4
^ permalink raw reply
* RE: Question re. git remote repository
From: Matt Seitz (matseitz) @ 2013-01-17 1:09 UTC (permalink / raw)
To: David Lang; +Cc: git@vger.kernel.org, ishchis2@gmail.com
In-Reply-To: <alpine.DEB.2.02.1301161617240.21503@nftneq.ynat.uz>
> From: David Lang [mailto:david@lang.hm]
>
> On Wed, 16 Jan 2013, Matt Seitz (matseitz) wrote:
>
> > Linus seemed to think it should work:
> >
> > http://permalink.gmane.org/gmane.comp.version-control.git/122670
>
> In the link you point at, he says that you can have problems with some
> types of
> actions. He points out things like git prune,
Linus wrote:
You do need to be a bit careful if you do maintenance operations
concurrently (I would suggest avoiding doing concurrent "git gc --prune",
for example), but any normal git workflow should be fine.
> but I would also say that there
> are probably race conditions if you have two git processes that try to
> change the HEAD to different things at the same time.
What makes you think there are race conditions?
Linus wrote:
And git doesn't have "proper locking", because it doesn't need it for
database ops: git objects are stable. For refs, git should be using the
proper NFS-safe "create and atomic rename" ops.
> > And "git init" specifically has a "shared" option:
> >
> > --shared[=(false|true|umask|group|all|world|everybody|0xxx)]
>
> I think this is dealing with multiple users _reading_ a repository, not
> making
> updates to it at the same time.
The description of "shared" says "This allows users belonging to the same group to push into that repository." The "push" command is about making updates.
^ permalink raw reply
* RE: Question re. git remote repository
From: David Lang @ 2013-01-17 1:26 UTC (permalink / raw)
To: Matt Seitz (matseitz); +Cc: git@vger.kernel.org, ishchis2@gmail.com
In-Reply-To: <A0DB01D693D8EF439496BC8B037A0AEF32209A54@xmb-rcd-x15.cisco.com>
On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote:
>> From: David Lang [mailto:david@lang.hm]
>>
>> On Wed, 16 Jan 2013, Matt Seitz (matseitz) wrote:
>>
>>> Linus seemed to think it should work:
>>>
>>> http://permalink.gmane.org/gmane.comp.version-control.git/122670
>>
>> In the link you point at, he says that you can have problems with some
>> types of
>> actions. He points out things like git prune,
>
> Linus wrote:
>
> You do need to be a bit careful if you do maintenance operations
> concurrently (I would suggest avoiding doing concurrent "git gc --prune",
> for example), but any normal git workflow should be fine.
>
>> but I would also say that there
>> are probably race conditions if you have two git processes that try to
>> change the HEAD to different things at the same time.
>
> What makes you think there are race conditions?
>
> Linus wrote:
>
> And git doesn't have "proper locking", because it doesn't need it for
> database ops: git objects are stable. For refs, git should be using the
> proper NFS-safe "create and atomic rename" ops.
As Linus points out, objects are stable, so when you create objects you don't
have to worry about locking, if two things write an object at the same time, the
same contents are being written so races don't matter.
However, if you have two people doing a commit or merge, you will get different
results based on the order they are happening in. This seems to be exactly the
type of thing that falls into the 'maintinance operations' category.
Linus says that git does not have "proper locking", so think about it, what do
you think will happen if person A does git add a/b; git commit and person B does
git add c/d; git commit?
Since the tree will look different depending on what order these four commands
execute in, the resulting HEAD could have multiple different values depending on
the order. The individual commits may even be different.
David Lang
>>> And "git init" specifically has a "shared" option:
>>>
>>> --shared[=(false|true|umask|group|all|world|everybody|0xxx)]
>>
>> I think this is dealing with multiple users _reading_ a repository, not
>> making
>> updates to it at the same time.
>
> The description of "shared" says "This allows users belonging to the same
> group to push into that repository." The "push" command is about making
> updates.
^ permalink raw reply
* RE: Question re. git remote repository
From: Matt Seitz (matseitz) @ 2013-01-17 1:46 UTC (permalink / raw)
To: David Lang; +Cc: git@vger.kernel.org, ishchis2@gmail.com
In-Reply-To: <alpine.DEB.2.02.1301161721110.21503@nftneq.ynat.uz>
> From: David Lang [mailto:david@lang.hm]
>
> Linus says that git does not have "proper locking", so think about it,
> what do
> you think will happen if person A does git add a/b; git commit and person
> B does
> git add c/d; git commit?
Sorry, I wasn't clear. My assumption is that a shared repository on a network file system will either be:
1. a bare repository that is normally accessed only by "git push" and "git pull" (or "git fetch"), the central repository model.
2. a repository where only one user does "git add" and "git commit", while other users will do "git pull", the peer-to-peer model (you pull changes from me, I pull changes from you).
^ permalink raw reply
* RE: Question re. git remote repository
From: David Lang @ 2013-01-17 1:59 UTC (permalink / raw)
To: Matt Seitz (matseitz); +Cc: git@vger.kernel.org, ishchis2@gmail.com
In-Reply-To: <A0DB01D693D8EF439496BC8B037A0AEF32209AD2@xmb-rcd-x15.cisco.com>
On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote:
>> From: David Lang [mailto:david@lang.hm]
>>
>> Linus says that git does not have "proper locking", so think about it,
>> what do
>> you think will happen if person A does git add a/b; git commit and person
>> B does
>> git add c/d; git commit?
>
> Sorry, I wasn't clear. My assumption is that a shared repository on a network file system will either be:
>
> 1. a bare repository that is normally accessed only by "git push" and "git pull" (or "git fetch"), the central repository model.
pulling from it would not be a problem, I could see issues with multiple pushes
taking place (the underlying repository would not get corrupted, but you will
very quickly hit conflicts where the push is not a fast forward and you need to
merge, not just push)
> 2. a repository where only one user does "git add" and "git commit", while
> other users will do "git pull", the peer-to-peer model (you pull changes from
> me, I pull changes from you).
At this point only one system is writing to the repository and it doesn't matter
that it's on network storage vs local storage.
pulling from a shared repository is probably safe, but I wouldn't bet against
there being any conditions where a pull at the same time someone is doing an
update being able to cause problems.
The normal thing is to do the pulls through git-daemon, and that does make sure
that what you are pulling is consistant.
David Lang
^ permalink raw reply
* Re: [PATCH v6 0/8] push: update remote tags only with force
From: Chris Rorvick @ 2013-01-17 2:19 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Max Horn, git, Angelo Borsotti, Drew Northup,
Michael Haggerty, Philip Oakley, Johannes Sixt, Kacper Kornet,
Felipe Contreras
In-Reply-To: <20130116174325.GA27525@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 11:43 AM, Jeff King <peff@peff.net> wrote:
> I think that is a reasonable rule that could be applied across all parts
> of the namespace hierarchy. And it could be applied by the client,
> because all you need to know is whether ref->old_sha1 is reachable from
> ref->new_sha1.
is_forwardable() did solve a UI issue. Previously all instances where
old is not reachable by new were assumed to be addressable with a
merge. is_forwardable() attempted to determine if the concept of
forwarding made sense given the inputs. For example, if old is a blob
it is useless to suggest merging it.
Chris
^ permalink raw reply
* RE: Question re. git remote repository
From: Matt Seitz (matseitz) @ 2013-01-17 2:25 UTC (permalink / raw)
To: David Lang; +Cc: git@vger.kernel.org, ishchis2@gmail.com
In-Reply-To: <alpine.DEB.2.02.1301161756240.21503@nftneq.ynat.uz>
> From: David Lang [mailto:david@lang.hm]
>
> On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote:
>
> > 1. a bare repository that is normally accessed only by "git push" and
> > "git pull" (or "git fetch"), the central repository model.
>
> pulling from it would not be a problem, I could see issues with multiple
> pushes taking place (the underlying repository would not get corrupted, but you
> will very quickly hit conflicts where the push is not a fast forward and you
> need to merge, not just push)
How is that different on a network file system, as opposed to using http, ssh, or git-daemon? Don't you get a "not a fast-forward" error, regardless of the protocol?
> > 2. a repository where only one user does "git add" and "git commit",
> while
> > other users will do "git pull", the peer-to-peer model (you pull changes
> from
> > me, I pull changes from you).
>
>
> pulling from a shared repository is probably safe, but I wouldn't bet
> against
> there being any conditions where a pull at the same time someone is doing
> an
> update being able to cause problems.
Why do you think there would be a problem?
> The normal thing is to do the pulls through git-daemon, and that does make
> sure
> that what you are pulling is consistant.
What does "git pull" via git-daemon do to ensure consistency that is different from "git pull" on a network file system?
^ permalink raw reply
* RE: Question re. git remote repository
From: David Lang @ 2013-01-17 2:49 UTC (permalink / raw)
To: Matt Seitz (matseitz); +Cc: git@vger.kernel.org, ishchis2@gmail.com
In-Reply-To: <A0DB01D693D8EF439496BC8B037A0AEF32209B45@xmb-rcd-x15.cisco.com>
On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote:
>> From: David Lang [mailto:david@lang.hm]
>>
>> On Thu, 17 Jan 2013, Matt Seitz (matseitz) wrote:
>>
>>> 1. a bare repository that is normally accessed only by "git push" and
>>> "git pull" (or "git fetch"), the central repository model.
>>
>> pulling from it would not be a problem, I could see issues with multiple
>> pushes taking place (the underlying repository would not get corrupted, but you
>> will very quickly hit conflicts where the push is not a fast forward and you
>> need to merge, not just push)
>
> How is that different on a network file system, as opposed to using http, ssh, or git-daemon? Don't you get a "not a fast-forward" error, regardless of the protocol?
true.
>>> 2. a repository where only one user does "git add" and "git commit",
>> while
>>> other users will do "git pull", the peer-to-peer model (you pull changes
>> from
>>> me, I pull changes from you).
>>
>>
>> pulling from a shared repository is probably safe, but I wouldn't bet
>> against
>> there being any conditions where a pull at the same time someone is doing
>> an
>> update being able to cause problems.
>
> Why do you think there would be a problem?
>
>> The normal thing is to do the pulls through git-daemon, and that does make
>> sure
>> that what you are pulling is consistant.
>
> What does "git pull" via git-daemon do to ensure consistency that is different from "git pull" on a network file system?
git pull via the daemon looks at what tree items you have on each end, and then
it sends you the items needed to make you match the server. If there are partial
updates on the server (due to some update in process) the daemon should not see
that, but if you are grabbing the files directly, I would be less confident that
you are always safe.
you may _be_ safe, and if others who really know the internals speak up, take
their word on it. But, absent assurances that we know that everything is done in
the right order in the face of a networked filesystem (which may break
visibility of changes due to caching), I would not trust such raw access for
updates at all, and only somewhat trust it for read-only use.
David Lang
^ permalink raw reply
* Re: [PATCH v6 0/8] push: update remote tags only with force
From: Jeff King @ 2013-01-17 3:11 UTC (permalink / raw)
To: Chris Rorvick
Cc: Junio C Hamano, Max Horn, git, Angelo Borsotti, Drew Northup,
Michael Haggerty, Philip Oakley, Johannes Sixt, Kacper Kornet,
Felipe Contreras
In-Reply-To: <CAEUsAPY8T9TYCrZLWB-0Mwae_NtnqqVvGwY+4jGfqh5Lh3=Dgw@mail.gmail.com>
On Wed, Jan 16, 2013 at 08:19:28PM -0600, Chris Rorvick wrote:
> On Wed, Jan 16, 2013 at 11:43 AM, Jeff King <peff@peff.net> wrote:
> > I think that is a reasonable rule that could be applied across all parts
> > of the namespace hierarchy. And it could be applied by the client,
> > because all you need to know is whether ref->old_sha1 is reachable from
> > ref->new_sha1.
>
> is_forwardable() did solve a UI issue. Previously all instances where
> old is not reachable by new were assumed to be addressable with a
> merge. is_forwardable() attempted to determine if the concept of
> forwarding made sense given the inputs. For example, if old is a blob
> it is useless to suggest merging it.
I think it makes sense to mark such a case as different from a regular
non-fast-forward (because "git pull" is not the right advice), but:
1. is_forwardable should assume a missing object is a commit not to
regress the common case; otherwise we do not show the pull advice
when we probably should, and most of the time it is going to be a
commit
2. When we know that we are not working with commits, I am not sure
that "already exists" is the right advice to give for such a case.
It is neither "this tag already exists, so we do not update it",
nor is it strictly "cannot fast forward this commit", but rather
something else.
The expanded definition of "what is a fast forward" that I
suggested would let this fall naturally between the two.
-Peff
^ permalink raw reply
* Re: [PATCH v6 0/8] push: update remote tags only with force
From: Chris Rorvick @ 2013-01-17 3:42 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Max Horn, git, Angelo Borsotti, Drew Northup,
Michael Haggerty, Philip Oakley, Johannes Sixt, Kacper Kornet,
Felipe Contreras
In-Reply-To: <20130117031100.GA7264@sigill.intra.peff.net>
On Wed, Jan 16, 2013 at 9:11 PM, Jeff King <peff@peff.net> wrote:
>> is_forwardable() did solve a UI issue. Previously all instances where
>> old is not reachable by new were assumed to be addressable with a
>> merge. is_forwardable() attempted to determine if the concept of
>> forwarding made sense given the inputs. For example, if old is a blob
>> it is useless to suggest merging it.
>
> I think it makes sense to mark such a case as different from a regular
> non-fast-forward (because "git pull" is not the right advice), but:
>
> 1. is_forwardable should assume a missing object is a commit not to
> regress the common case; otherwise we do not show the pull advice
> when we probably should, and most of the time it is going to be a
> commit
Yes, obviously this was a bug, thus the use of "attempted" above. It
would have been better to assume a missing 'old' was potentially
forwardable to present the user with the most helpful advice.
> 2. When we know that we are not working with commits, I am not sure
> that "already exists" is the right advice to give for such a case.
> It is neither "this tag already exists, so we do not update it",
> nor is it strictly "cannot fast forward this commit", but rather
> something else.
But the reference already existing in the remote is a substantial
reason for not allowing the push in all of these cases. You can break
this out further if you like to explain why the specific reference
shouldn't be moved on the remote, but this is even more complicated a
simple "is old reachable from new?" test.
Chris
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox