Re: Bug: .gitconfig folder
On Wed, May 27, 2015 at 03:24:47PM -0700, Stefan Beller wrote: > What is our thinking for after-fact recovery attempts? > Like we try the mmap first, if that fails we just use open to get the > contents of > the file. And when open fails, we can still print a nice error message? For config, I think we could just open and read the file in the first place. The data is not typically very big (and if you have a 3G config file and git barfs with "out of memory", I can live with that). -Peff -- 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: Redirect "git" subcommand to itself?
On Wed, May 27, 2015 at 06:53:26PM -0700, Junio C Hamano wrote: > > I wonder if we want to make a "git" subcommand, which behaves exactly > > the same as git itself? > > Then "git git git status" would just return the same as "git status". > > A few unrelated thoughts. > > * Perhaps we should omit 'git' from these advice-texts? E.g. > > use "revert --abort" to cancel > >I dunno. Please don't. You help the set of people who type "git" and then paste the rest of the command at the expense of people who just paste the whole command. We don't know the relative numbers of those people, but we know there is at least 1 in each group. ;) -Peff -- 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: Redirect "git" subcommand to itself?
Stefan Beller writes: > so I just run into this problem again (which happens to me maybe twice a > week): > I want to do a git operations, so I type "git " into my shell, and > then I look around what > exactly I want to do and usually I find it in the help text of a > previous command such as > You are currently reverting commit 383c14b. > (fix conflicts and run "git revert --continue") > (use "git revert --abort" to cancel the revert operation) > > then I copy the whole operation "git revert --abort" in this case and > paste it to the shell > and let go. > The result looks like > $ git git revert --abort > git: 'git' is not a git command. See 'git --help'. > > Did you mean this? > init > > I wonder if we want to make a "git" subcommand, which behaves exactly > the same as git itself? > Then "git git git status" would just return the same as "git status". A few unrelated thoughts. * Perhaps we should omit 'git' from these advice-texts? E.g. use "revert --abort" to cancel I dunno. * While we bend over backwards to a certain degree to be helpful, I somehow feel making "git git" a synonym to "git" is going too far, akin to asking POSIX maintainers to define "act", "cta", "atc", "tca", and "tac" all as synonyms to "cat" because you often fat-finger when typing "cat" (yes, "tac" does something else that is more useful, I know). * You can help yourself with something like this, I suppose: [alias] git = "!sh -c 'exec git \"$@\"' -" but I personally feel that it is too ugly to live as part of our official suggestion, so please do not send a patch to add it as a built-in alias ;-). -- 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] glossary: add "remote", "submodule", "superproject"
Noticed-by: Philip Oakley Helped-by: Junio C Hamano Signed-off-by: Stefan Beller --- Documentation/glossary-content.txt | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index bf383c2..23ab692 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -469,6 +469,11 @@ The most notable example is `HEAD`. <> to describe the mapping between remote <> and local ref. +[[def_remote]]remote repository:: + A <> which is used to track the same + project but resides somewhere else. To communicate with remotes, + see <> or <>. + [[def_remote_tracking_branch]]remote-tracking branch:: A <> that is used to follow changes from another <>. It typically looks like @@ -515,6 +520,18 @@ The most notable example is `HEAD`. is created by giving the `--depth` option to linkgit:git-clone[1], and its history can be later deepened with linkgit:git-fetch[1]. +[[def_submodule]]submodule:: + A <> that holds the history of a + separate project inside another repository (the latter of + which is called <>). The + containing superproject knows about the names of (but does + not hold copies of) commit objects of the contained submodules. + +[[def_superproject]]superproject:: + A <> that references other repositories + inside itself as <>. The superproject + tracks only the remote and the name of the submodule. + [[def_symref]]symref:: Symbolic reference: instead of containing the <> id itself, it is of the format 'ref: refs/some/thing' and when -- 2.4.1.345.gab207b6.dirty -- 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
Redirect "git" subcommand to itself?
Hi, so I just run into this problem again (which happens to me maybe twice a week): I want to do a git operations, so I type "git " into my shell, and then I look around what exactly I want to do and usually I find it in the help text of a previous command such as You are currently reverting commit 383c14b. (fix conflicts and run "git revert --continue") (use "git revert --abort" to cancel the revert operation) then I copy the whole operation "git revert --abort" in this case and paste it to the shell and let go. The result looks like $ git git revert --abort git: 'git' is not a git command. See 'git --help'. Did you mean this? init I wonder if we want to make a "git" subcommand, which behaves exactly the same as git itself? Then "git git git status" would just return the same as "git status". Thanks, Stefan -- 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 1/2] git-rebase -i: Add key word "drop" to remove a commit
From: "Junio C Hamano" Matthieu Moy writes: I find it weird to write noop True, but then it can be spelled # too, so do we still want 'drop'? Unless we have a strong reason to believe migrants from Hg cannot be (re)trained, personally, I'd feel that we do not need this 'drop' thing. To me, the addition of "drop" would be a better completion of the list of action verbs for 'normal' users. Training/Retraining users to use atypical techniques is a never ending task, so making drop a synonym for the existing noop appeals to my experience of users (of all sorts of tools, including personal experience ;-). -- Philip -- 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] glossary: add "remote" and "submodule"
On Wed, May 27, 2015 at 4:05 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
+[[def_submodule]]submodule::
+ A <> inside another repository. The two
+ repositories have different history, though the outer repository
+ knows the commit of the inner repository.
>>>
>> ... But correctness trumps brevity indeed.
>
> I do not think the correct way is that much longer, though.
>
> A repository inside another repository. The two repositories have different
> history
> A repository that holds the history of a separate project inside another
> repository
>
> Heh, they are the same length, no?
>
>>
>>>
>>>A repository that holds the history of a separate project
>>>inside another repository (the latter of which is called
>>>superproject).
>>
>> This is better than what I proposed, but confusing. When naming
>> a project a submodule, my mental standpoint is the superproject.
>> ("This project has the submodule foo and bar"). But In your description
>> the superproject is called "another repository".
>
> That is because you are adding an entry for "submodule" to the
> glossary, no? I was writing from submodule's point of view, i.e. "I
> (submodule) is inside another repository, and my project is separate
> from that other repository's".
The submodule doesn't know it's a submodule though, so the point of view
"I (as a submodule)" only happens rarely in the real world?
I have a library in mind when talking about submodules. And the libraries
maintainer may not care if their library is used as a submodule or just
"make install"ed or just put somewhere in the filesystem
Usually submodules are only interesting from the superprojects point of view,
like "I want to upgrade libfoo now, so I make a commit changing the gitlink
of the submodule to point at that tag/commit"
That's why I found the presented perspective a bit strange.
>
>>>The containing superproject knows about the
>>>names of (but does not hold copies of) commit objects of the
>>>contained submodules.
>>
>> That makes sense to point out here. Though should we also introduce
>> "superproject" now?
>
> Yes, that is what I was hinting at.
ok
--
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] p4: Retrieve the right revision of the UTF-16 file
Fixing bug with UTF-16 files when they are retrieved by git-p4.
It was always getting the tip version of the file and the history of the
file was lost.
Signed-off-by: Miguel Torroja
---
git-p4.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index cdfa2df..be2c7da 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap):
# them back too. This is not needed to the cygwin windows version,
# just the native "NT" type.
#
-text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
+text = p4_read_pipe(['print', '-q', '-o', '-', "%s@%s" %
(file['depotFile'], file['change']) ])
if p4_version_string().find("/NT") >= 0:
text = text.replace("\r\n", "\n")
contents = [ text ]
--
1.7.10.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] glossary: add "remote" and "submodule"
Stefan Beller writes:
>>> +[[def_submodule]]submodule::
>>> + A <> inside another repository. The two
>>> + repositories have different history, though the outer repository
>>> + knows the commit of the inner repository.
>>
> ... But correctness trumps brevity indeed.
I do not think the correct way is that much longer, though.
A repository inside another repository. The two repositories have different
history
A repository that holds the history of a separate project inside another
repository
Heh, they are the same length, no?
>
>>
>>A repository that holds the history of a separate project
>>inside another repository (the latter of which is called
>>superproject).
>
> This is better than what I proposed, but confusing. When naming
> a project a submodule, my mental standpoint is the superproject.
> ("This project has the submodule foo and bar"). But In your description
> the superproject is called "another repository".
That is because you are adding an entry for "submodule" to the
glossary, no? I was writing from submodule's point of view, i.e. "I
(submodule) is inside another repository, and my project is separate
from that other repository's".
>>The containing superproject knows about the
>>names of (but does not hold copies of) commit objects of the
>>contained submodules.
>
> That makes sense to point out here. Though should we also introduce
> "superproject" now?
Yes, that is what I was hinting at.
--
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] glossary: add "remote" and "submodule"
On Wed, May 27, 2015 at 3:29 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> Noticed-by: Philip Oakley
>> Signed-off-by: Stefan Beller
>> ---
>> Documentation/glossary-content.txt | 10 ++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/Documentation/glossary-content.txt
>> b/Documentation/glossary-content.txt
>> index bf383c2..e303135 100644
>> --- a/Documentation/glossary-content.txt
>> +++ b/Documentation/glossary-content.txt
>> @@ -469,6 +469,11 @@ The most notable example is `HEAD`.
>> <> to describe the mapping between remote
>> <> and local ref.
>>
>> +[[def_remote]]remote repository::
>> + A <> which is used to track the same
>> + project but resides somewhere else. To communicate with remotes,
>> + see <> or <>.
>> +
>
> OK.
>
>> @@ -515,6 +520,11 @@ The most notable example is `HEAD`.
>> is created by giving the `--depth` option to linkgit:git-clone[1], and
>> its history can be later deepened with linkgit:git-fetch[1].
>>
>> +[[def_submodule]]submodule::
>> + A <> inside another repository. The two
>> + repositories have different history, though the outer repository
>> + knows the commit of the inner repository.
>
> I'd stress that they are not just different histories (as the
> 'master' and the 'maint' branches of my project has different
> histories) but they are separate projects. Perhaps like this?
This is a very subtle distinction IMHO, as both master and maint
"are the same project". Looking from enough distance, it's just the
git project without the fine detail of what makes these 2 histories different.
I tried coming up with a short paragraph, which may explain my choice
of words. But correctness trumps brevity indeed.
>
>A repository that holds the history of a separate project
>inside another repository (the latter of which is called
>superproject).
This is better than what I proposed, but confusing. When naming
a project a submodule, my mental standpoint is the superproject.
("This project has the submodule foo and bar"). But In your description
the superproject is called "another repository".
>The containing superproject knows about the
>names of (but does not hold copies of) commit objects of the
>contained submodules.
That makes sense to point out here. Though should we also introduce
"superproject" now?
>
> It is not like that it is strange or unintuitive that the
> superproject knows about some commits in its submodule. "X, though
> Y" however makes it sound as if Y is true "despite X". I do not
> think there is any "despite" here.
--
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: Bug: .gitconfig folder
Jeff King writes:
>> -die_errno("Out of memory? mmap failed");
>> +die_errno("mmap failed");
>
> This is definitely an improvement, but the real failing of that error
> message is that it does not tell us that "~/.gitconfig" is the culprit.
> I don't think we can do much from xmmap, though; it does not have the
> filename. It would be nice if we got EISDIR from open() in the first
> place, but I don't think we can implement that efficiently (if we added
> an "xopen" that checked that, it would have to stat() every file we
> opened).
The patch was meant to be a tongue-in-cheek tangent that is a vast
improvement for cases where we absolutely need to use mmap but does
not help the OP at all ;-) I do not think there is any need for the
config reader to read the existing file via mmap interface; just
open it, strbuf_read() the whole thing (and complain when it cannot)
and we should be ok.
Or do we write back through the mmaped region or something?
--
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] glossary: add "remote" and "submodule"
Stefan Beller writes: > Noticed-by: Philip Oakley > Signed-off-by: Stefan Beller > --- > Documentation/glossary-content.txt | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/glossary-content.txt > b/Documentation/glossary-content.txt > index bf383c2..e303135 100644 > --- a/Documentation/glossary-content.txt > +++ b/Documentation/glossary-content.txt > @@ -469,6 +469,11 @@ The most notable example is `HEAD`. > <> to describe the mapping between remote > <> and local ref. > > +[[def_remote]]remote repository:: > + A <> which is used to track the same > + project but resides somewhere else. To communicate with remotes, > + see <> or <>. > + OK. > @@ -515,6 +520,11 @@ The most notable example is `HEAD`. > is created by giving the `--depth` option to linkgit:git-clone[1], and > its history can be later deepened with linkgit:git-fetch[1]. > > +[[def_submodule]]submodule:: > + A <> inside another repository. The two > + repositories have different history, though the outer repository > + knows the commit of the inner repository. I'd stress that they are not just different histories (as the 'master' and the 'maint' branches of my project has different histories) but they are separate projects. Perhaps like this? A repository that holds the history of a separate project inside another repository (the latter of which is called superproject). The containing superproject knows about the names of (but does not hold copies of) commit objects of the contained submodules. It is not like that it is strange or unintuitive that the superproject knows about some commits in its submodule. "X, though Y" however makes it sound as if Y is true "despite X". I do not think there is any "despite" here. -- 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: Bug: .gitconfig folder
On Wed, May 27, 2015 at 3:18 PM, Jeff King wrote:
> On Wed, May 27, 2015 at 01:30:29PM -0700, Junio C Hamano wrote:
>
>> Jorge writes:
>>
>> > If you have a folder named ~/.gitconfig instead of a file with that
>> > name, when you try to run some global config editing command it will
>> > fail with a wrong error message:
>> >
>> > "fatal: Out of memory? mmap failed: No such device"
>>
>> That indeed is a funny error message.
>>
>> How about this patch?
>>
>> -- >8 --
>> We show that message with die_errno(), but the OS is ought to know
>> why mmap(2) failed much better than we do. There is no reason for
>> us to say "Out of memory?" here.
>>
>> Note that mmap(2) fails with ENODEV when the file you specify is not
>> something that can be mmap'ed, so you still need to know that "No
>> such device" can include cases like having a directory when a
>> regular file is expected, but we can expect that a user who creates
>> a directory to a location where a regular file is expected to be
>> would know what s/he is doing, hopefully ;-)
>>
>> sha1_file.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sha1_file.c b/sha1_file.c
>> index ccc6dac..551a9e9 100644
>> --- a/sha1_file.c
>> +++ b/sha1_file.c
>> @@ -720,7 +720,7 @@ void *xmmap(void *start, size_t length,
>> release_pack_memory(length);
>> ret = mmap(start, length, prot, flags, fd, offset);
>> if (ret == MAP_FAILED)
>> - die_errno("Out of memory? mmap failed");
>> + die_errno("mmap failed");
>> }
>
> This is definitely an improvement, but the real failing of that error
> message is that it does not tell us that "~/.gitconfig" is the culprit.
> I don't think we can do much from xmmap, though; it does not have the
> filename. It would be nice if we got EISDIR from open() in the first
> place, but I don't think we can implement that efficiently (if we added
> an "xopen" that checked that, it would have to stat() every file we
> opened).
What is our thinking for after-fact recovery attempts?
Like we try the mmap first, if that fails we just use open to get the
contents of
the file. And when open fails, we can still print a nice error message?
>
> -Peff
> --
> 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
--
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] config: add options to list only variable names
SZEDER Gábor writes:
> diff --git a/builtin/config.c b/builtin/config.c
> index 7188405..38bcf83 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -13,6 +13,7 @@ static char *key;
> static regex_t *key_regexp;
> static regex_t *regexp;
> static int show_keys;
> +static int show_only_keys;
Is it just me who thinks this is a strange phrase? Somehow these
words do not roll well on my tongue.
Perhaps "static int omit_values"? Which would match what this part
does pretty well:
> static int show_all_config(const char *key_, const char *value_, void *cb)
> {
> - if (value_)
> + if (!show_only_keys && value_)
That is, "if not omitting values and there is a value, then do this".
> - return format_config(&values->items[values->nr++], key_, value_);
> + if (show_only_keys) {
> + struct strbuf *buf = &values->items[values->nr++];
> + strbuf_init(buf, 0);
> + strbuf_addstr(buf, key_);
> + strbuf_addch(buf, term);
> + return 0;
xstrfmt()?
--
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: Bug: .gitconfig folder
On Wed, May 27, 2015 at 01:30:29PM -0700, Junio C Hamano wrote:
> Jorge writes:
>
> > If you have a folder named ~/.gitconfig instead of a file with that
> > name, when you try to run some global config editing command it will
> > fail with a wrong error message:
> >
> > "fatal: Out of memory? mmap failed: No such device"
>
> That indeed is a funny error message.
>
> How about this patch?
>
> -- >8 --
> We show that message with die_errno(), but the OS is ought to know
> why mmap(2) failed much better than we do. There is no reason for
> us to say "Out of memory?" here.
>
> Note that mmap(2) fails with ENODEV when the file you specify is not
> something that can be mmap'ed, so you still need to know that "No
> such device" can include cases like having a directory when a
> regular file is expected, but we can expect that a user who creates
> a directory to a location where a regular file is expected to be
> would know what s/he is doing, hopefully ;-)
>
> sha1_file.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index ccc6dac..551a9e9 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -720,7 +720,7 @@ void *xmmap(void *start, size_t length,
> release_pack_memory(length);
> ret = mmap(start, length, prot, flags, fd, offset);
> if (ret == MAP_FAILED)
> - die_errno("Out of memory? mmap failed");
> + die_errno("mmap failed");
> }
This is definitely an improvement, but the real failing of that error
message is that it does not tell us that "~/.gitconfig" is the culprit.
I don't think we can do much from xmmap, though; it does not have the
filename. It would be nice if we got EISDIR from open() in the first
place, but I don't think we can implement that efficiently (if we added
an "xopen" that checked that, it would have to stat() every file we
opened).
-Peff
--
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/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
On Wed, May 27, 2015 at 01:44:26PM -0700, Junio C Hamano wrote:
> Paul Tan writes:
>
> > @@ -17,6 +34,10 @@ struct am_state {
> > struct strbuf dir;/* state directory path */
> > int cur; /* current patch number */
> > int last; /* last patch number */
> > + struct strbuf msg;/* commit message */
> > + struct strbuf author_name;/* commit author's name */
> > + struct strbuf author_email; /* commit author's email */
> > + struct strbuf author_date;/* commit author's date */
> > int prec; /* number of digits in patch filename */
> > };
>
> I always get suspicious when structure fields are overly commented,
> wondering if it is a sign of naming fields poorly. All of the above
> fields look quite self-explanatory and I am not sure if it is worth
> having these comments, spending efforts to type SP many times to
> align them and all.
Just my 2 cents, but I think that grouping and overhead comments can
often make things more obvious. For example:
struct am_state {
/* state directory path */
struct strbuf dir;
/*
* current and last patch numbers; are these 1-indexed
* or 0-indexed?
*/
int cur;
int last;
struct strbuf author_name;
struct strbuf author_email;
struct strbuf author_date;
struct strbuf msg;
/* number of digits in patch filename */
int prec;
}
Note that by grouping "cur" and "last", we can discuss them as a group,
and the overhead comment gives us room to mention their shared
properties (my indexing question is a real question, btw, whose answer I
think would be useful to mention in a comment).
Likewise, by grouping the "msg" strbuf with the author information, it
becomes much more clear that they are all about the commit metadata
(though I would not be opposed to a comment above the whole block,
either).
-Peff
--
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/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
Paul Tan writes:
> +static const char *msgnum(const struct am_state *state)
> +{
> + static struct strbuf fmt = STRBUF_INIT;
> + static struct strbuf sb = STRBUF_INIT;
> +
> + strbuf_reset(&fmt);
> + strbuf_addf(&fmt, "%%0%dd", state->prec);
> +
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, fmt.buf, state->cur);
Hmph, wouldn't ("%*d", state->prec, state->cur) work or am I missing
something?
--
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
Release candidate of Git for Windows 2.x is out
Hi all,
I just uploaded release candidates for the upcoming Git for Windows 2.x
release. Please find the download link here:
https://git-for-windows.github.io/#download
There are 32-bit and 64-bit versions both of regular installers and portable
installers ("portable" meaning that they are .7z archives that can be unpacked
anywhere and run in place, without any need for running an installer).
My projected time line is to hammer out the last kinks until Friday, and then
continue after a one-week leave, if needed, and then finally retire msysGit and
start the official 2.x release cycle of Git for Windows.
If you are running Windows and have a little time to spare, please test this
release candidate thoroughly. If you find bugs, please first look at
https://github.com/git-for-windows/git/issues (even the closed ones), and
comment either on existing tickets or open new ones. It would be even cooler,
of course, if you could open Pull Requests with fixes :-)
Ciao,
Johannes
--
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] p4: Retrieve the right revision of the UTF-16 file
On Wed, May 27, 2015 at 3:04 PM, Luke Diamand wrote:
> On 27/05/15 23:31, Miguel Torroja wrote:
>>
>> Fixing bug with UTF-16 files when they are retreived by git-p4.
>> It was always getting the tip version of the file and the history of the
>> file was lost.
>
> This looks sensible to me, and seems to work in some simple testing, thanks!
>
> Ack.
>
> Luke
Thanks; Miguel, please sign-off your patch; otherwise we cannot use it.
Thanks.
>> ---
>> git-p4.py |2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index cdfa2df..be2c7da 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap):
>> # them back too. This is not needed to the cygwin windows
>> version,
>> # just the native "NT" type.
>> #
>> -text = p4_read_pipe(['print', '-q', '-o', '-',
>> file['depotFile']])
>> +text = p4_read_pipe(['print', '-q', '-o', '-', "%s@%s" %
>> (file['depotFile'], file['change']) ])
>> if p4_version_string().find("/NT") >= 0:
>> text = text.replace("\r\n", "\n")
>> contents = [ text ]
>>
>
--
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] p4: Retrieve the right revision of the UTF-16 file
On 27/05/15 23:31, Miguel Torroja wrote:
Fixing bug with UTF-16 files when they are retreived by git-p4.
It was always getting the tip version of the file and the history of the
file was lost.
This looks sensible to me, and seems to work in some simple testing, thanks!
Ack.
Luke
---
git-p4.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index cdfa2df..be2c7da 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap):
# them back too. This is not needed to the cygwin windows
version,
# just the native "NT" type.
#
-text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
+text = p4_read_pipe(['print', '-q', '-o', '-', "%s@%s" %
(file['depotFile'], file['change']) ])
if p4_version_string().find("/NT") >= 0:
text = text.replace("\r\n", "\n")
contents = [ text ]
--
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/WIP 2/8] wrapper: implement xfopen()
On Wed, May 27, 2015 at 09:33:32PM +0800, Paul Tan wrote:
> +/**
> + * xfopen() is the same as fopen(), but it die()s if the fopen() fails.
> + */
> +FILE *xfopen(const char *path, const char *mode)
> +{
> + FILE *fp;
> +
> + assert(path);
> + assert(mode);
> + fp = fopen(path, mode);
> + if (!fp) {
> + if (*mode == 'w' || *mode == 'a')
> + die_errno(_("could not open '%s' for writing"), path);
This misses "r+". I don't think we use that in our code currently, but
if we're going to introduce a wrapper like this, I think it makes sense
to cover all cases.
-Peff
--
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] Documentation: include 'merge.branchdesc' for merge and config as well
'merge.branchdesc' is only mentioned in the docs of 'git fmt-merge-msg'. The description of 'merge.log' is already duplicated between 'merge-config.txt' and 'git-fmt-merge-msg.txt'; instead of duplicating the description of another config variable, extract the descriptions of both of these variables from 'git-fmt-merge-msg.txt' into a separate file and include it there and in 'merge-config.txt'. Leave 'merge.summary' only in git-fmt-merge-msg.txt, as it is marked for deprecation. Signed-off-by: SZEDER Gábor --- Documentation/fmt-merge-msg-config.txt | 10 ++ Documentation/git-fmt-merge-msg.txt| 12 +--- Documentation/merge-config.txt | 6 +- 3 files changed, 12 insertions(+), 16 deletions(-) create mode 100644 Documentation/fmt-merge-msg-config.txt diff --git a/Documentation/fmt-merge-msg-config.txt b/Documentation/fmt-merge-msg-config.txt new file mode 100644 index 00..c73cfa90b7 --- /dev/null +++ b/Documentation/fmt-merge-msg-config.txt @@ -0,0 +1,10 @@ +merge.branchdesc:: + In addition to branch names, populate the log message with + the branch description text associated with them. Defaults + to false. + +merge.log:: + In addition to branch names, populate the log message with at + most the specified number of one-line descriptions from the + actual commits that are being merged. Defaults to false, and + true is a synonym for 20. diff --git a/Documentation/git-fmt-merge-msg.txt b/Documentation/git-fmt-merge-msg.txt index bb1232a52c..55a9a4b93a 100644 --- a/Documentation/git-fmt-merge-msg.txt +++ b/Documentation/git-fmt-merge-msg.txt @@ -51,17 +51,7 @@ OPTIONS CONFIGURATION - - -merge.branchdesc:: - In addition to branch names, populate the log message with - the branch description text associated with them. Defaults - to false. - -merge.log:: - In addition to branch names, populate the log message with at - most the specified number of one-line descriptions from the - actual commits that are being merged. Defaults to false, and - true is a synonym for 20. +include::fmt-merge-msg-config.txt[] merge.summary:: Synonym to `merge.log`; this is deprecated and will be removed in diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 8a0e52f8ee..002ca58c21 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -26,11 +26,7 @@ merge.ff:: allowed (equivalent to giving the `--ff-only` option from the command line). -merge.log:: - In addition to branch names, populate the log message with at - most the specified number of one-line descriptions from the - actual commits that are being merged. Defaults to false, and - true is a synonym for 20. +include::fmt-merge-msg-config.txt[] merge.renameLimit:: The number of files to consider when performing rename detection -- 2.4.2.349.g6883b65 -- 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/WIP 1/8] wrapper: implement xopen()
On Wed, May 27, 2015 at 09:03:47PM +0200, Torsten Bögershausen wrote:
> The original open can take 2 or 3 parameters, how about this:
> int xopen(const char *path, int oflag, ... )
> {
> va_list params;
> int mode;
> int fd;
>
> va_start(params, oflag);
> mode = va_arg(params, int);
> va_end(params);
>
> fd = open(path, oflag, mode);
Don't you need a conditional on pulling the mode arg off the stack
(i.e., if O_CREAT is in the flags)?
-Peff
--
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 1/2] git-rebase -i: Add key word "drop" to remove a commit
On Wed, May 27, 2015 at 1:35 PM, Junio C Hamano wrote: > Matthieu Moy writes: > >> Junio C Hamano writes: >> >>> Matthieu Moy writes: >>> I find it weird to write noop >>> >>> True, but then it can be spelled >>> >>> # >> >> I do find it weird too. "#" means "comment", which means "do as if it >> was not there" to me. And in this case it does change the semantics once >> you activate the safety feature: error out without the "# >> ", rebase dropping the commit if the comment is present. > > Well, I do not agree with the premise that "A line was removed, the > user may have made a mistake, we need to warn about it" is a good > idea in the first place. Removing an insn that is not wanted has > been the way to skip and not replay a change from the beginning of > the time, and users shouldn't be trained into thinking that somehow > is a bad practice by having such an option that warns. Talking about ideas: I sometimes have the wrong branch checked out when doing a small fixup commit. So I want to drop that patch from the current branch and apply it to another branch. Maybe an instruction like cherry-pick-to-branch-(and-do-not-apply-here) would help me there. On the other hand I do understand the reasoning for having more safety features in rebase as that exposes lots of power and many people find the power a bit daunting. So maybe you don't want to check the rebase instructions, but rather after the fact, when the rebase is done: $ git rebase -i origin/master Successfully rebased and updated refs/heads/mytopic Rebased the following commits: 0e33744 Document protocol version 2 6b6e3a7 t5544: add a test case for the new protocol d6aff73 transport: get_refs_via_connect exchanges capabilities before refs. cbb6089 transport: connect_setup appends protocol version number 0b86fa1 fetch-pack: use the configured transport protocol 23ed0ff remote.h: add get_remote_capabilities, request_capabilities e18b6dc transport: add infrastructure to support a protocol version number fd8d40d upload-pack-2: Implement the version 2 of upload-pack bf781ae upload-pack: move capabilities out of send_ref 4c9cb59 upload-pack: make client capability parsing code a separate function Dropped the following commits: deadbee upload-pack: only accept capabilities on the first "want" line New commits: (due to rewording, double picking, etc) c0ffee1 More Documentation I'd guess you would construct the information from the reflog (The line before "rebase -i (start)" in the reflog) delta'd against HEAD, so it's a crude incantation of git log maybe? Also we need to turn this off for the power users, though I'd welcome if we'd make it default on in git 3. (Being maximally verbose is good for new users I assume, and turning it off is easy for advanced folks, so we can do that for all porcelain commands?) > > -- > 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 -- 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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 01:45:55PM -0700, Stefan Beller wrote: > > Right, but I think (and please correct me if there's a case I'm > > missing) that the behavior is the same whether it is spelled > > "ping-pong" or "capability:ping-pong". That is, the rule for > > "capability:" is "if you do not understand it, ignore it and do not > > mention it in your capabilities; the server is required to assume > > you were written before that capability was invented". But that is > > _also_ the rule for ping-pong, I think. > > The rules are the same, right. But the allowed characters are limited > (in theory) as the regular expressions given for the capabilities > don't allow for binary data for example, but only well formed ASCII > text, space separated. The "ping-pong" keyword could introduce a > binary stream there including line feeds. (Today it sounds like a > stupid idea though) Do we need that restriction? IOW, as long as we parse from the start of the packet and give up as soon as we realize it is not a thing we understand, I do not think anybody is hurt by the contents of the item. E.g., if an old client sees: 00XXping-pong= It knows: 1. The item starts with ping-pong; we don't know what that is, so we never even bother looking at the binary goo. 2. It's in a pkt-line. We read to the end of the packet line and throw the rest of the data away. Now we're synchronized to read the next item. Of course, "ping-pong" may also introduce another phase to the protocol which is not encapsulated in pkt-lines (e.g., if the data is too big to fit right inside the capability pkt-line, or if it needs a lot of back and forth like ref negotiation). But then we only proceed to that phase if both sides have said "I understand ping-pong". So I think we are capable of boot-strapping just about anything using capability negotiation (with the exception of fixing the capability negotiation itself; but even that, we can bootstrap a second more intensive capability negotiation using a capability in the initial list). -Peff -- 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] config: add options to list only variable names
Quoting Jeff King : On Wed, May 27, 2015 at 10:07:19PM +0200, SZEDER Gábor wrote: Help the completion script by introducing the '--list-names' and '--get-names-regexp' options, the "names-only" equivalents of '--list' and '--get-regexp', so it doesn't have to separate variable names from their values anymore. Thanks, this sounds like the best solution. It should be a tiny bit more efficient, too, though I doubt it matters much in practice. -'git config' [] [-z|--null] -l | --list +'git config' [] [-z|--null] -l | --list | --list-name s/list-name/&s/, to match the code (and your commit message). And note how I added an extra 's' to the other option in the commit message! cat > expect << EOF +beta.noindent +nextsection.nonewline +123456.a123 +version.1.2.3eX.alpha +EOF + +test_expect_success 'working --list-names' ' + git config --list-names > output && + test_cmp expect output +' + +cat > expect << EOF We usually avoid the extra space after redirection operators. But we also usually match existing code. I'm not sure which is more evil in this case. ;) +test_expect_success '--get-name-regexp' ' + git config --get-name-regexp in >output && + test_cmp expect output +' This one is the odd man out if you are following existing style, though. Heh, in both cases I simply copied the existing "name-less" test, and only adjusted the expected output and the name of the option to test. :) Will reroll. Gábor -- 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 3/5] verify_lock(): report errors via a strbuf
On 05/27/2015 09:48 PM, Junio C Hamano wrote: > Michael Haggerty writes: > >> Instead of writing error messages directly to stderr, write them to a >> "strbuf *err". In lock_ref_sha1_basic(), arrange for these errors to >> be returned to its caller. > > I had to scratch my head and view long outside the context before > realizing that the caller lock_ref_sha1_basic() already arranges > with its caller that errors from it are passed via the strbuf, and > this change is just turning verify_lock(), a callee from it, follows > that already established pattern. > > Looks sensible, but the last sentence was misleading at least to me. > > The caller, lock_ref_sha1_basic(), uses this error reporting > convention with all the other callees, and reports its error > this way to its callers. > > perhaps? +1 Thanks for clarifying this sentence. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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] glossary: add "remote" and "submodule"
Noticed-by: Philip Oakley Signed-off-by: Stefan Beller --- Documentation/glossary-content.txt | 10 ++ 1 file changed, 10 insertions(+) diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt index bf383c2..e303135 100644 --- a/Documentation/glossary-content.txt +++ b/Documentation/glossary-content.txt @@ -469,6 +469,11 @@ The most notable example is `HEAD`. <> to describe the mapping between remote <> and local ref. +[[def_remote]]remote repository:: + A <> which is used to track the same + project but resides somewhere else. To communicate with remotes, + see <> or <>. + [[def_remote_tracking_branch]]remote-tracking branch:: A <> that is used to follow changes from another <>. It typically looks like @@ -515,6 +520,11 @@ The most notable example is `HEAD`. is created by giving the `--depth` option to linkgit:git-clone[1], and its history can be later deepened with linkgit:git-fetch[1]. +[[def_submodule]]submodule:: + A <> inside another repository. The two + repositories have different history, though the outer repository + knows the commit of the inner repository. + [[def_symref]]symref:: Symbolic reference: instead of containing the <> id itself, it is of the format 'ref: refs/some/thing' and when -- 2.4.1.345.gab207b6.dirty -- 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] config: add options to list only variable names
On Wed, May 27, 2015 at 05:04:38PM -0400, Jeff King wrote: > > -'git config' [] [-z|--null] -l | --list > > +'git config' [] [-z|--null] -l | --list | --list-name > > s/list-name/&s/, to match the code (and your commit message). Doh, just saw your "1.5". FWIW, I expected "PATCH 1.5/2" to be "eh, I should have put this in between patches 1 and 2". I expect a re-roll to be "v1.5" (or just "v2"). This was the only real error in the patch, so your 1.5 looks OK to me. Though I hope you will consider my other suggestions, too. -Peff -- 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 2/2] completion: use new 'git config' options to reliably list variable names
On Wed, May 27, 2015 at 10:07:20PM +0200, SZEDER Gábor wrote: > List all set config variable names with 'git config --list-names' instead > of '--list' post processing. Similarly, use 'git config > --get-name-regexp' instead of '--get-regexp' to get config variables in a > given section. > > Signed-off-by: SZEDER Gábor > --- > contrib/completion/git-completion.bash | 15 +++ > 1 file changed, 3 insertions(+), 12 deletions(-) Nice diffstat. The less hacky bash parsing we can do, the better. -Peff -- 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] config: add options to list only variable names
On Wed, May 27, 2015 at 10:07:19PM +0200, SZEDER Gábor wrote:
> Help the completion script by introducing the '--list-names' and
> '--get-names-regexp' options, the "names-only" equivalents of '--list' and
> '--get-regexp', so it doesn't have to separate variable names from their
> values anymore.
Thanks, this sounds like the best solution. It should be a tiny bit more
efficient, too, though I doubt it matters much in practice.
> -'git config' [] [-z|--null] -l | --list
> +'git config' [] [-z|--null] -l | --list | --list-name
s/list-name/&s/, to match the code (and your commit message).
> @@ -161,6 +166,9 @@ See also <>.
> --list::
> List all variables set in config file.
>
> +--list-name::
> + List the names of all variables set in config file.
Ditto here. Also, now that we have two similar modes, perhaps the
"--list" description above should become:
List all variables set in config file, along with their values.
> @@ -165,7 +170,14 @@ static int collect_config(const char *key_, const char
> *value_, void *cb)
>
> ALLOC_GROW(values->items, values->nr + 1, values->alloc);
>
> - return format_config(&values->items[values->nr++], key_, value_);
> + if (show_only_keys) {
> + struct strbuf *buf = &values->items[values->nr++];
> + strbuf_init(buf, 0);
> + strbuf_addstr(buf, key_);
> + strbuf_addch(buf, term);
> + return 0;
> + } else
> + return format_config(&values->items[values->nr++], key_,
> value_);
> }
Might it flow a little better to always enter format_config, and then
just return early (before writing the value) when show_key_only is set?
> cat > expect << EOF
> +beta.noindent
> +nextsection.nonewline
> +123456.a123
> +version.1.2.3eX.alpha
> +EOF
> +
> +test_expect_success 'working --list-names' '
> + git config --list-names > output &&
> + test_cmp expect output
> +'
> +
> +cat > expect << EOF
We usually avoid the extra space after redirection operators. But we
also usually match existing code. I'm not sure which is more evil in
this case. ;)
> +test_expect_success '--get-name-regexp' '
> + git config --get-name-regexp in >output &&
> + test_cmp expect output
> +'
This one is the odd man out if you are following existing style,
though.
The rest of the patch looks good to me.
-Peff
--
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/4] showing existing ws breakage
Jeff King writes:
> On Wed, May 27, 2015 at 01:46:26PM -0700, Junio C Hamano wrote:
>
>> Jeff King writes:
>>
>> > Do you want me to also eradicate PLAIN from the code itself? It's a
>> > rather simple change, but it does touch a lot of places.
>>
>> Nah, that is not user-facing. We do not do s/cache/index/ in the
>> code, either.
>>
>> Besides, I actually find plain much easier to type than context ;-)
>
> OK. I just did it while our emails were in flight, so here it is just
> for reference.
I actually like that; the changes are fairly isolated and contained.
>
> -- >8 --
> Subject: diff.h: rename DIFF_PLAIN color slot to DIFF_CONTEXT
>
> The latter is a much more descriptive name (and we support
> "color.diff.context" now). This also updates the name of any
> local variables which were used to store the color.
>
> Signed-off-by: Jeff King
> ---
> combine-diff.c | 6 +++---
> diff.c | 26 +-
> diff.h | 2 +-
> line-log.c | 6 +++---
> 4 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/combine-diff.c b/combine-diff.c
> index 8eb7278..30c7eb6 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -730,7 +730,7 @@ static void dump_sline(struct sline *sline, const char
> *line_prefix,
> const char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);
> const char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);
> const char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);
> - const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
> + const char *c_context = diff_get_color(use_color, DIFF_CONTEXT);
> const char *c_reset = diff_get_color(use_color, DIFF_RESET);
>
> if (result_deleted)
> @@ -793,7 +793,7 @@ static void dump_sline(struct sline *sline, const char
> *line_prefix,
> }
> if (comment_end)
> printf("%s%s %s%s", c_reset,
> - c_plain, c_reset,
> + c_context, c_reset,
> c_func);
> for (i = 0; i < comment_end; i++)
> putchar(hunk_comment[i]);
> @@ -828,7 +828,7 @@ static void dump_sline(struct sline *sline, const char
> *line_prefix,
>*/
> if (!context)
> continue;
> - fputs(c_plain, stdout);
> + fputs(c_context, stdout);
> }
> else
> fputs(c_new, stdout);
> diff --git a/diff.c b/diff.c
> index 27bd371..100773f 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -42,7 +42,7 @@ static long diff_algorithm;
>
> static char diff_colors[][COLOR_MAXLEN] = {
> GIT_COLOR_RESET,
> - GIT_COLOR_NORMAL, /* PLAIN */
> + GIT_COLOR_NORMAL, /* CONTEXT */
> GIT_COLOR_BOLD, /* METAINFO */
> GIT_COLOR_CYAN, /* FRAGINFO */
> GIT_COLOR_RED, /* OLD */
> @@ -55,7 +55,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
> static int parse_diff_color_slot(const char *var)
> {
> if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
> - return DIFF_PLAIN;
> + return DIFF_CONTEXT;
> if (!strcasecmp(var, "meta"))
> return DIFF_METAINFO;
> if (!strcasecmp(var, "frag"))
> @@ -501,7 +501,7 @@ static void emit_add_line(const char *reset,
> static void emit_hunk_header(struct emit_callback *ecbdata,
>const char *line, int len)
> {
> - const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
> + const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
> const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
> const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
> const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
> @@ -518,7 +518,7 @@ static void emit_hunk_header(struct emit_callback
> *ecbdata,
> if (len < 10 ||
> memcmp(line, atat, 2) ||
> !(ep = memmem(line + 2, len - 2, atat, 2))) {
> - emit_line(ecbdata->opt, plain, reset, line, len);
> + emit_line(ecbdata->opt, context, reset, line, len);
> return;
> }
> ep += 2; /* skip over @@ */
> @@ -540,7 +540,7 @@ static void emit_hunk_header(struct emit_callback
> *ecbdata,
> if (*ep != ' ' && *ep != '\t')
> break;
> if (ep != cp) {
> - strbuf_addstr(&msgbuf, plain);
> + strbuf_addstr(&msgbuf, context);
> strbuf_add(&msgbuf, cp, ep - cp);
> strbuf_addstr(&msgbuf, reset);
> }
> @@ -623,10 +623,10 @@ static void emit_rewrite_lines(struct emit_callb
Re: [PATCH v3 0/4] showing existing ws breakage
On Wed, May 27, 2015 at 03:22:18AM -0400, Jeff King wrote: > In color.diff.*, these are called "new", "old", and "plain". I am of the > opinion that "context" is a far better name than "plain", but perhaps we > should support both for consistency. > > Here's a patch for the color.diff side, if we want to go that route. So I had originally thought we would support both names in both places. But since the diff patch we ended up with is basically "the real name is context; plain is just a historical anomaly", I do not see any need to support "plain" in your new whitespace code. I suspect you came to the same conclusion independently, but I wanted to make sure what I had written before didn't leave anybody confused. -Peff -- 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/4] showing existing ws breakage
On Wed, May 27, 2015 at 01:46:26PM -0700, Junio C Hamano wrote:
> Jeff King writes:
>
> > Do you want me to also eradicate PLAIN from the code itself? It's a
> > rather simple change, but it does touch a lot of places.
>
> Nah, that is not user-facing. We do not do s/cache/index/ in the
> code, either.
>
> Besides, I actually find plain much easier to type than context ;-)
OK. I just did it while our emails were in flight, so here it is just
for reference.
-- >8 --
Subject: diff.h: rename DIFF_PLAIN color slot to DIFF_CONTEXT
The latter is a much more descriptive name (and we support
"color.diff.context" now). This also updates the name of any
local variables which were used to store the color.
Signed-off-by: Jeff King
---
combine-diff.c | 6 +++---
diff.c | 26 +-
diff.h | 2 +-
line-log.c | 6 +++---
4 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 8eb7278..30c7eb6 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -730,7 +730,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
const char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);
const char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);
const char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);
- const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
+ const char *c_context = diff_get_color(use_color, DIFF_CONTEXT);
const char *c_reset = diff_get_color(use_color, DIFF_RESET);
if (result_deleted)
@@ -793,7 +793,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
}
if (comment_end)
printf("%s%s %s%s", c_reset,
- c_plain, c_reset,
+ c_context, c_reset,
c_func);
for (i = 0; i < comment_end; i++)
putchar(hunk_comment[i]);
@@ -828,7 +828,7 @@ static void dump_sline(struct sline *sline, const char
*line_prefix,
*/
if (!context)
continue;
- fputs(c_plain, stdout);
+ fputs(c_context, stdout);
}
else
fputs(c_new, stdout);
diff --git a/diff.c b/diff.c
index 27bd371..100773f 100644
--- a/diff.c
+++ b/diff.c
@@ -42,7 +42,7 @@ static long diff_algorithm;
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
- GIT_COLOR_NORMAL, /* PLAIN */
+ GIT_COLOR_NORMAL, /* CONTEXT */
GIT_COLOR_BOLD, /* METAINFO */
GIT_COLOR_CYAN, /* FRAGINFO */
GIT_COLOR_RED, /* OLD */
@@ -55,7 +55,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
static int parse_diff_color_slot(const char *var)
{
if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
- return DIFF_PLAIN;
+ return DIFF_CONTEXT;
if (!strcasecmp(var, "meta"))
return DIFF_METAINFO;
if (!strcasecmp(var, "frag"))
@@ -501,7 +501,7 @@ static void emit_add_line(const char *reset,
static void emit_hunk_header(struct emit_callback *ecbdata,
const char *line, int len)
{
- const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
+ const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
@@ -518,7 +518,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
if (len < 10 ||
memcmp(line, atat, 2) ||
!(ep = memmem(line + 2, len - 2, atat, 2))) {
- emit_line(ecbdata->opt, plain, reset, line, len);
+ emit_line(ecbdata->opt, context, reset, line, len);
return;
}
ep += 2; /* skip over @@ */
@@ -540,7 +540,7 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
if (*ep != ' ' && *ep != '\t')
break;
if (ep != cp) {
- strbuf_addstr(&msgbuf, plain);
+ strbuf_addstr(&msgbuf, context);
strbuf_add(&msgbuf, cp, ep - cp);
strbuf_addstr(&msgbuf, reset);
}
@@ -623,10 +623,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
data += len;
}
if (!endp) {
- const char *plain = diff_get_color(ecb->color_diff,
- DIFF_PL
Re: [RFC/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 1:34 PM, Jeff King wrote: > On Wed, May 27, 2015 at 10:40:37AM -0700, Stefan Beller wrote: > >> > If we are upload-pack-2, should we advertise that in the capabilities? I >> > think it may make things easier later if we try to provide some >> > opportunistic out-of-band data. E.g., if see tell git-daemon: >> > >> > git-upload-pack repo\0host=whatever\0\0version=2 >> > >> > how do we know whether version=2 was understood and kicked us into v2 >> > mode, versus an old server that ignored it? >> >> So in my vision we would call git-upload-pack-2 instead of having a >> "version=2". >> and if git-upload-pack-2 doesn't exist, the whole conversation is >> over, the client >> it is left to make up some good error message or retry version 1. > > I'd like for that to be a starting point for us, and then to be able to > add-on "hints" to ease the transition path in whatever way we want. We > may even not do that in the long run, but I want to leave the door open > if we can. > >> But I think advertising both which versions the server could deal >> with, as well as >> the currently expected version is a good thing. >> >> capability: can_speak=1,2 >> capability: speaking_now=2 > > I was thinking just "speaking_now=2", but it probably makes sense to do > both. I do not think using it to "downgrade" will ever be particularly > useful (certainly not from v2 to v1, since to understand the flag both > sides must be v2 in the first place). But advertising that via the v1 > conversation will be a good way to tell the other side that upgrade is > possible. If for some reason we discover a flaw in the current version, which makes it unusable (a buffer overflow?, some stupid abuse which makes the capability list huge), you may want to force downgrading (and in the very distant future when we are current on version 4 and have dropped version 1 already, you can only downgrade to 2 and 3, so I can see value in it. Another idea to make it all more future proof: "capability: speaking_now=2" must be sent as the first line, so then you can adapt on the client side easily for which version you are listening. > >> > Also, do we need the capability noise-word? >> >> I thought it opens up a new possible door in the future. >> As we ignore anything not starting with "capability" for now, you >> could introduce >> your foo and bar ping pong easily and still be version 2 compatible. >> >> S: capability: thin >> S: capability: another-capability >> S: ping-pong foo >> S: done >> C: (not having understood ping-pong) just answering with capability: thin >> C: done, let's proceed to refs advertisement >> >> The alternative client would do: >> >> C: ping-pong: foo-data1a >> S: ping-pong: foo-data1b >> C: ping-pong: foo-data2a >> C: capability: thin >> ... > > Right, but I think (and please correct me if there's a case I'm missing) > that the behavior is the same whether it is spelled "ping-pong" or > "capability:ping-pong". That is, the rule for "capability:" is "if you > do not understand it, ignore it and do not mention it in your > capabilities; the server is required to assume you were written before > that capability was invented". But that is _also_ the rule for > ping-pong, I think. The rules are the same, right. But the allowed characters are limited (in theory) as the regular expressions given for the capabilities don't allow for binary data for example, but only well formed ASCII text, space separated. The "ping-pong" keyword could introduce a binary stream there including line feeds. (Today it sounds like a stupid idea though) > >> > Eric mentioned the underflow problems here, but I wonder even more: >> > what's wrong with the global ends_with() that we already provide? >> >> I was missing knowledge we have that, and apparently I was thinking it's >> faster to come up with my own version than to look for it. :) > > Makes sense. :) > > -Peff -- 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/4] showing existing ws breakage
Jeff King writes: > Do you want me to also eradicate PLAIN from the code itself? It's a > rather simple change, but it does touch a lot of places. Nah, that is not user-facing. We do not do s/cache/index/ in the code, either. Besides, I actually find plain much easier to type than context ;-) -- 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/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
Paul Tan writes:
> @@ -17,6 +34,10 @@ struct am_state {
> struct strbuf dir;/* state directory path */
> int cur; /* current patch number */
> int last; /* last patch number */
> + struct strbuf msg;/* commit message */
> + struct strbuf author_name;/* commit author's name */
> + struct strbuf author_email; /* commit author's email */
> + struct strbuf author_date;/* commit author's date */
> int prec; /* number of digits in patch filename */
> };
I always get suspicious when structure fields are overly commented,
wondering if it is a sign of naming fields poorly. All of the above
fields look quite self-explanatory and I am not sure if it is worth
having these comments, spending efforts to type SP many times to
align them and all.
By the way, the overall structure of the series look sensible.
> +static int read_author_script(struct am_state *state)
> +{
> + char *value;
> + struct strbuf sb = STRBUF_INIT;
> + const char *filename = am_path(state, "author-script");
> + FILE *fp = fopen(filename, "r");
> + if (!fp) {
> + if (errno == ENOENT)
> + return 0;
> + die(_("could not open '%s' for reading"), filename);
Hmph, do we want to report with die_errno()?
> + }
> +
> + if (strbuf_getline(&sb, fp, '\n'))
> + return -1;
> + if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))
This cast is unfortunate; can't "value" be of "const char *" type?
--
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/WIP 3/8] am: implement patch queue mechanism
Paul Tan writes: > Makefile | 2 +- > builtin.h| 1 + > builtin/am.c | 167 > +++ > git.c| 1 + > 4 files changed, 170 insertions(+), 1 deletion(-) > create mode 100644 builtin/am.c > > diff --git a/Makefile b/Makefile > index 323c401..57a7c8c 100644 > --- a/Makefile > +++ b/Makefile > @@ -466,7 +466,6 @@ TEST_PROGRAMS_NEED_X = > # interactive shell sessions without exporting it. > unexport CDPATH > > -SCRIPT_SH += git-am.sh If you are building this "new am" incrementally (and for something complex like "am", that's the only sensible way), perhaps it is a good idea to do the "competing implementation" trick I suggested earlier when we were discussing your "new pull" patches, to allow you to keep both versions and switch between them at runtime. That way, you can run tests, both existing ones and new ones you add, on both versions to make sure they behave the same way. -- 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/4] showing existing ws breakage
On Wed, May 27, 2015 at 11:57:15AM -0700, Junio C Hamano wrote: > > -- >8 -- > > Subject: diff: accept color.diff.context as a synonym for "plain" > > > > The term "plain" is a bit ambiguous; let's allow the more > > specific "context", but keep "plain" around for > > compatibility. > > > > Signed-off-by: Jeff King > > --- > > I didn't bother mentioning the historical "plain" in the documentation. > > I don't know if it's better to (for people who find it in the wild and > > wonder what it means) or if it simply clutters the description. > > 'plain' does sound a misnomer, as these slot names are about "what" > are painted, not "how" they are painted. The latter is what their > values represent. Whoever named that slot was confused by the fact > that 'context' (i.e. "what") lines are by default painted in 'plain' > color without frills (i.e. "how"). > > We usually try to give a brief mention to historical names primarily > to silence those who pick up stale information from the Web, get > curious, and then complain loudly after finding that we no longer > document them even though we keep accepting them silently, so I am > somewhat tempted to do this on top. Yeah, I waffled on doing it myself. If you take the patch, please do squash that in. Do you want me to also eradicate PLAIN from the code itself? It's a rather simple change, but it does touch a lot of places. -Peff -- 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 1/2] git-rebase -i: Add key word "drop" to remove a commit
Matthieu Moy writes: > Junio C Hamano writes: > >> Matthieu Moy writes: >> >>> I find it weird to write >>> >>> noop >> >> True, but then it can be spelled >> >> # > > I do find it weird too. "#" means "comment", which means "do as if it > was not there" to me. And in this case it does change the semantics once > you activate the safety feature: error out without the "# > ", rebase dropping the commit if the comment is present. Well, I do not agree with the premise that "A line was removed, the user may have made a mistake, we need to warn about it" is a good idea in the first place. Removing an insn that is not wanted has been the way to skip and not replay a change from the beginning of the time, and users shouldn't be trained into thinking that somehow is a bad practice by having such an option that warns. -- 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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 10:40:37AM -0700, Stefan Beller wrote: > > If we are upload-pack-2, should we advertise that in the capabilities? I > > think it may make things easier later if we try to provide some > > opportunistic out-of-band data. E.g., if see tell git-daemon: > > > > git-upload-pack repo\0host=whatever\0\0version=2 > > > > how do we know whether version=2 was understood and kicked us into v2 > > mode, versus an old server that ignored it? > > So in my vision we would call git-upload-pack-2 instead of having a > "version=2". > and if git-upload-pack-2 doesn't exist, the whole conversation is > over, the client > it is left to make up some good error message or retry version 1. I'd like for that to be a starting point for us, and then to be able to add-on "hints" to ease the transition path in whatever way we want. We may even not do that in the long run, but I want to leave the door open if we can. > But I think advertising both which versions the server could deal > with, as well as > the currently expected version is a good thing. > > capability: can_speak=1,2 > capability: speaking_now=2 I was thinking just "speaking_now=2", but it probably makes sense to do both. I do not think using it to "downgrade" will ever be particularly useful (certainly not from v2 to v1, since to understand the flag both sides must be v2 in the first place). But advertising that via the v1 conversation will be a good way to tell the other side that upgrade is possible. > > Also, do we need the capability noise-word? > > I thought it opens up a new possible door in the future. > As we ignore anything not starting with "capability" for now, you > could introduce > your foo and bar ping pong easily and still be version 2 compatible. > > S: capability: thin > S: capability: another-capability > S: ping-pong foo > S: done > C: (not having understood ping-pong) just answering with capability: thin > C: done, let's proceed to refs advertisement > > The alternative client would do: > > C: ping-pong: foo-data1a > S: ping-pong: foo-data1b > C: ping-pong: foo-data2a > C: capability: thin > ... Right, but I think (and please correct me if there's a case I'm missing) that the behavior is the same whether it is spelled "ping-pong" or "capability:ping-pong". That is, the rule for "capability:" is "if you do not understand it, ignore it and do not mention it in your capabilities; the server is required to assume you were written before that capability was invented". But that is _also_ the rule for ping-pong, I think. > > Eric mentioned the underflow problems here, but I wonder even more: > > what's wrong with the global ends_with() that we already provide? > > I was missing knowledge we have that, and apparently I was thinking it's > faster to come up with my own version than to look for it. :) Makes sense. :) -Peff -- 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] p4: Retrieve the right revision of the UTF-16 file
Fixing bug with UTF-16 files when they are retreived by git-p4.
It was always getting the tip version of the file and the history of the
file was lost.
---
git-p4.py |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index cdfa2df..be2c7da 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2098,7 +2098,7 @@ class P4Sync(Command, P4UserMap):
# them back too. This is not needed to the cygwin windows version,
# just the native "NT" type.
#
-text = p4_read_pipe(['print', '-q', '-o', '-', file['depotFile']])
+text = p4_read_pipe(['print', '-q', '-o', '-', "%s@%s" %
(file['depotFile'], file['change']) ])
if p4_version_string().find("/NT") >= 0:
text = text.replace("\r\n", "\n")
contents = [ text ]
--
1.7.10.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: Bug: .gitconfig folder
Jorge writes:
> If you have a folder named ~/.gitconfig instead of a file with that
> name, when you try to run some global config editing command it will
> fail with a wrong error message:
>
> "fatal: Out of memory? mmap failed: No such device"
That indeed is a funny error message.
How about this patch?
-- >8 --
We show that message with die_errno(), but the OS is ought to know
why mmap(2) failed much better than we do. There is no reason for
us to say "Out of memory?" here.
Note that mmap(2) fails with ENODEV when the file you specify is not
something that can be mmap'ed, so you still need to know that "No
such device" can include cases like having a directory when a
regular file is expected, but we can expect that a user who creates
a directory to a location where a regular file is expected to be
would know what s/he is doing, hopefully ;-)
sha1_file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sha1_file.c b/sha1_file.c
index ccc6dac..551a9e9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -720,7 +720,7 @@ void *xmmap(void *start, size_t length,
release_pack_memory(length);
ret = mmap(start, length, prot, flags, fd, offset);
if (ret == MAP_FAILED)
- die_errno("Out of memory? mmap failed");
+ die_errno("mmap failed");
}
return ret;
}
--
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/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number
On Wed, May 27, 2015 at 12:01:50PM -0700, Stefan Beller wrote:
> > Interesting choice for the short option ("-v" would be nice, but
> > obviously it is taken). Do we want to delay on claiming the
> > short-and-sweet 'y' until we are sure this is something people will use
> > a lot? In an ideal world, it is not (i.e., auto-upgrade and other tricks
> > become good enough that nobody bothers to specify it manually).
> [...]
> Or do you rather hint on dropping the short option at all, and just having
> NULL
> in the field?
Yes, that's what I was hinting.
-Peff
--
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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 01:30:28PM -0400, Eric Sunshine wrote:
> > Like Eric, I find the whole next_capability thing a little ugly. His
> > suggestion to pass in the parsing state is an improvement, but I wonder
> > why we need to parse at all. Can we keep the capabilities as:
> >
> > const char *capabilities[] = {
> > "multi_ack",
> > "thin-pack",
> > ... etc ...
> > };
> >
> > and then loop over the array?
>
> Yes, that would be much nicer. I also had this in mind but didn't know
> if there was a strong reason for the capabilities to be shoehorned
> into a single string as they are currently.
I don't think there is a good reason, beyond it being the simplest thing
for the current code to work. But as you can see from the existing
packet_write() in upload-pack, it's already going through some
contortions to handle optional capabilities (i.e., "capabilities" is by
no means the full list anymore).
Doing it item by item will mean we can't use a single packet_write() in
the v1 code, but it's OK to format into a buffer here (we already need
such a buffer for format_symref_info anyway).
-Peff
--
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/2] config: add options to list only variable names
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:
$ git config --get branch.b.description
Branch description to fool the completion script with a
second line containing dot . and equals = characters.
$ git config --unset
...
second line containing dot . and equals
...
The completion script runs 'git config --list' and processes its output to
strip the values and keep only the variable names. It does so by looking
for lines containing '.' and '=' and outputting everything before the '=',
which was fooled by my multi-line branch description.
A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.
Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.
Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Signed-off-by: SZEDER Gábor
---
D'oh, misspelled the option in the docs...
Documentation/git-config.txt | 10 +-
builtin/config.c | 22 ++
contrib/completion/git-completion.bash | 4 ++--
t/t1300-repo-config.sh | 22 ++
4 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096faa..fd519f81d8 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
'git config' [] [type] [-z|--null] --get-all name [value_regex]
'git config' [] [type] [-z|--null] --get-regexp name_regex
[value_regex]
'git config' [] [type] [-z|--null] --get-urlmatch name URL
+'git config' [] [-z|--null] --get-name-regexp name_regex
'git config' [] --unset name [value_regex]
'git config' [] --unset-all name [value_regex]
'git config' [] --rename-section old_name new_name
'git config' [] --remove-section name
-'git config' [] [-z|--null] -l | --list
+'git config' [] [-z|--null] -l | --list | --list-names
'git config' [] --get-color name [default]
'git config' [] --get-colorbool name [stdout-is-tty]
'git config' [] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
in which section and variable names are lowercased, but subsection
names are not.
+--get-name-regexp::
+ Like --get-regexp, but shows only matching variable names, not its
+ values.
+
--get-urlmatch name URL::
When given a two-part name section.key, the value for
section..key whose part matches the best to the
@@ -161,6 +166,9 @@ See also <>.
--list::
List all variables set in config file.
+--list-names::
+ List the names of all variables set in config file.
+
--bool::
'git config' will ensure that the output is "true" or "false"
diff --git a/builtin/config.c b/builtin/config.c
index 7188405f7e..38bcf838c5 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int show_only_keys;
static int use_key_regexp;
static int do_all;
static int do_not_match;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
#define ACTION_GET_COLOR (1<<13)
#define ACTION_GET_COLORBOOL (1<<14)
#define ACTION_GET_URLMATCH (1<<15)
+#define ACTION_LIST_NAMES (1<<16)
+#define ACTION_GET_NAME_REGEXP (1<<17)
#define TYPE_BOOL (1<<0)
#define TYPE_INT (1<<1)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"),
ACTION_GET),
OPT_BIT(0, "get-all", &actions, N_("get all values: key
[value-regex]"), ACTION_GET_ALL),
OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp:
name-regex [value-regex]"), ACTION_GET_REGEXP),
+ OPT_BIT(0, "get-name-regexp", &actions, N_("get names for regexp:
name-regex"), ACTION_GET_NAME_REGEXP),
OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the
URL: section[.var] URL"), ACTION_GET_URLMATCH),
OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables:
name value [value_regex]"), ACTION_REPLACE_ALL),
OPT_BIT(0, "add", &actions, N_("add a new variable: name value"),
ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name
new-name"), ACTION_RENAME_SECTION),
OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"),
ACTION_REMOVE_SECTION),
OPT_BIT('l', "list",
Re: [RFC/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities
On Wed, May 27, 2015 at 01:19:39PM -0400, Eric Sunshine wrote: > >> The 'len > 4' check is needed because there's no guarantee that 'line' > >> is NUL-terminated. Correct? > > > > I think this was just blindly copied from get_remote_heads(). And I > > think that code was being overly paranoid. Ever since f3a3214 (Make > > send/receive-pack be closer to doing something interesting, 2005-06-29), > > the pkt-line reader will add an extra NUL to the buffer to ease cases > > like this. > > Thanks. I had started digging into packet_read() to determine whether > it guaranteed NUL-termination, but didn't get far enough to decide. I > agree that if NUL-termination is guaranteed, then the 'len > 4' check > is superfluous (and confusing, which is why it caught my attention in > the first place). Yeah, agreed that it should be cleaned up. Interestingly, if you dig on that line, I've touched it several times myself and never noticed this. :) -Peff -- 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] send-email: Add sendmail email aliases format
Allen Hubbe writes: > Add support for the sendmail email aliases format. Thanks. > ** Note: A general 'where to find documentation' paragraph will be added >by Junio, appearing either before or after this patch in the series. You didn't have to do this to me; as long as you agree with me that the paragraph is a good thing to have, it is OK (and even more preferable) to include it in this patch. That's called collaboration. If other person's contribution was really significant and the change can stand on its own, then a split two-patch series with the author set to the other person may not be a bad idea, and if other person's contribution was really significant but the change by the other person cannot stand on its own, "Helped-by" in the log message would be sufficient. My contribution in this case is much less than that. > ** Note: A fix to other tests to eliminate the use of tilde for the home >dir will be added by Junio, appearing either before or after this >patch in the series. That is a sensible thing to do, as it does not relate to this change. Thanks. Will queue and let's start merging this topic to 'next' and down. -- 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] config: add options to list only variable names
Recenty I created a multi-line branch description with '.' and '='
characters on one of the lines, and noticed that fragments of that line
show up when completing set variable names for 'git config', e.g.:
$ git config --get branch.b.description
Branch description to fool the completion script with a
second line containing dot . and equals = characters.
$ git config --unset
...
second line containing dot . and equals
...
The completion script runs 'git config --list' and processes its output to
strip the values and keep only the variable names. It does so by looking
for lines containing '.' and '=' and outputting everything before the '=',
which was fooled by my multi-line branch description.
A similar issue exists with aliases and pretty format aliases with
multi-line values, but in that case 'git config --get-regexp' is run and
subsequent lines don't have to contain either '.' or '=' to fool the
completion script.
Though 'git config' can produce null-terminated output for newline-safe
parsing, that's of no use in this case, becase we can't cope with nulls in
the shell.
Help the completion script by introducing the '--list-names' and
'--get-names-regexp' options, the "names-only" equivalents of '--list' and
'--get-regexp', so it doesn't have to separate variable names from their
values anymore.
Signed-off-by: SZEDER Gábor
---
Documentation/git-config.txt | 10 +-
builtin/config.c | 22 ++
contrib/completion/git-completion.bash | 4 ++--
t/t1300-repo-config.sh | 22 ++
4 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 02ec096..e0d27d5 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -16,11 +16,12 @@ SYNOPSIS
'git config' [] [type] [-z|--null] --get-all name [value_regex]
'git config' [] [type] [-z|--null] --get-regexp name_regex
[value_regex]
'git config' [] [type] [-z|--null] --get-urlmatch name URL
+'git config' [] [-z|--null] --get-name-regexp name_regex
'git config' [] --unset name [value_regex]
'git config' [] --unset-all name [value_regex]
'git config' [] --rename-section old_name new_name
'git config' [] --remove-section name
-'git config' [] [-z|--null] -l | --list
+'git config' [] [-z|--null] -l | --list | --list-name
'git config' [] --get-color name [default]
'git config' [] --get-colorbool name [stdout-is-tty]
'git config' [] -e | --edit
@@ -96,6 +97,10 @@ OPTIONS
in which section and variable names are lowercased, but subsection
names are not.
+--get-name-regexp::
+ Like --get-regexp, but shows only matching variable names, not its
+ values.
+
--get-urlmatch name URL::
When given a two-part name section.key, the value for
section..key whose part matches the best to the
@@ -161,6 +166,9 @@ See also <>.
--list::
List all variables set in config file.
+--list-name::
+ List the names of all variables set in config file.
+
--bool::
'git config' will ensure that the output is "true" or "false"
diff --git a/builtin/config.c b/builtin/config.c
index 7188405..38bcf83 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -13,6 +13,7 @@ static char *key;
static regex_t *key_regexp;
static regex_t *regexp;
static int show_keys;
+static int show_only_keys;
static int use_key_regexp;
static int do_all;
static int do_not_match;
@@ -43,6 +44,8 @@ static int respect_includes = -1;
#define ACTION_GET_COLOR (1<<13)
#define ACTION_GET_COLORBOOL (1<<14)
#define ACTION_GET_URLMATCH (1<<15)
+#define ACTION_LIST_NAMES (1<<16)
+#define ACTION_GET_NAME_REGEXP (1<<17)
#define TYPE_BOOL (1<<0)
#define TYPE_INT (1<<1)
@@ -60,6 +63,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"),
ACTION_GET),
OPT_BIT(0, "get-all", &actions, N_("get all values: key
[value-regex]"), ACTION_GET_ALL),
OPT_BIT(0, "get-regexp", &actions, N_("get values for regexp:
name-regex [value-regex]"), ACTION_GET_REGEXP),
+ OPT_BIT(0, "get-name-regexp", &actions, N_("get names for regexp:
name-regex"), ACTION_GET_NAME_REGEXP),
OPT_BIT(0, "get-urlmatch", &actions, N_("get value specific for the
URL: section[.var] URL"), ACTION_GET_URLMATCH),
OPT_BIT(0, "replace-all", &actions, N_("replace all matching variables:
name value [value_regex]"), ACTION_REPLACE_ALL),
OPT_BIT(0, "add", &actions, N_("add a new variable: name value"),
ACTION_ADD),
@@ -68,6 +72,7 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name
new-name"), ACTION_RENAME_SECTION),
OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"),
ACTION_REMOVE_SECTION),
OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
+ OPT_BIT(0,
[PATCH 2/2] completion: use new 'git config' options to reliably list variable names
List all set config variable names with 'git config --list-names' instead
of '--list' post processing. Similarly, use 'git config
--get-name-regexp' instead of '--get-regexp' to get config variables in a
given section.
Signed-off-by: SZEDER Gábor
---
contrib/completion/git-completion.bash | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 6abbd56..121aa31 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -744,9 +744,8 @@ __git_compute_porcelain_commands ()
__git_get_config_variables ()
{
local section="$1" i IFS=$'\n'
- for i in $(git --git-dir="$(__gitdir)" config --get-regexp
"^$section\..*" 2>/dev/null); do
- i="${i#$section.}"
- echo "${i/ */}"
+ for i in $(git --git-dir="$(__gitdir)" config --get-name-regexp
"^$section\..*" 2>/dev/null); do
+ echo "${i#$section.}"
done
}
@@ -1774,15 +1773,7 @@ __git_config_get_set_variables ()
c=$((--c))
done
- git --git-dir="$(__gitdir)" config $config_file --list 2>/dev/null |
- while read -r line
- do
- case "$line" in
- *.*=*)
- echo "${line/=*/}"
- ;;
- esac
- done
+ git --git-dir="$(__gitdir)" config $config_file --list-names 2>/dev/null
}
_git_config ()
--
2.4.2.347.ge926c0d
--
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 verify_lock() to report errors via strbuf
Michael Haggerty writes: > The last sentence is nonsense. This patch series relies on > lock_ref_sha1_basic() having a "strbuf *err" parameter, which is only > the case since > > 4a32b2e lock_ref_sha1_basic(): report errors via a "struct strbuf > *err" (2015-05-11) > > The latter commit is in mh/ref-directory-file (which has now been merged > to master, so technically the last sentence is now correct again). [5/5] seems to conflict with the write_ref_sha1() vs write_ref_to_lockfile() updates; I think I can manage, though ;-) -- 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 3/5] verify_lock(): report errors via a strbuf
Michael Haggerty writes: > Instead of writing error messages directly to stderr, write them to a > "strbuf *err". In lock_ref_sha1_basic(), arrange for these errors to > be returned to its caller. I had to scratch my head and view long outside the context before realizing that the caller lock_ref_sha1_basic() already arranges with its caller that errors from it are passed via the strbuf, and this change is just turning verify_lock(), a callee from it, follows that already established pattern. Looks sensible, but the last sentence was misleading at least to me. The caller, lock_ref_sha1_basic(), uses this error reporting convention with all the other callees, and reports its error this way to its callers. perhaps? -- 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
[PATCHv3] submodule documentation: Reorder introductory paragraphs
It's better to start the man page with a description of what submodules actually are instead of saying what they are not. Reorder the paragraphs such that the first short paragraph introduces the submodule concept, the second paragraph highlights the usage of the submodule command, the third paragraph giving background information, and finally the fourth paragraph discusing alternatives such as subtrees and remotes, which we don't want to be confused with. This ordering deepens the knowledge on submodules with each paragraph. First the basic questions like "How/what" will be answered, while the underlying concepts will be taught at a later time. Making sure it is not confused with subtrees and remotes is not really enhancing knowledge of submodules itself, but rather painting the big picture of git concepts, so you could also argue to have it as the second paragraph. Personally I think this may confuse readers, specially newcomers though. Additionally to reordering the paragraphs, they have been slightly reworded. Signed-off-by: Stefan Beller --- I think this is the best I can come up with for now. * It still mentions the remotes as a potential explanation mud-hole, but I feel it helps the reader understand submodules a little better. * We also start with a typical git man page intro (Dropping "This command does ...") Documentation/git-submodule.txt | 50 ++--- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 2c25916..2ca1391 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -25,22 +25,17 @@ SYNOPSIS DESCRIPTION --- -Submodules allow foreign repositories to be embedded within -a dedicated subdirectory of the source tree, always pointed -at a particular commit. +Inspects, updates and manages submodules. -They are not to be confused with remotes, which are meant mainly -for branches of the same project; submodules are meant for -different projects you would like to make part of your source tree, -while the history of the two projects still stays completely -independent and you cannot modify the contents of the submodule -from within the main project. -If you want to merge the project histories and want to treat the -aggregated whole as a single project from then on, you may want to -add a remote for the other project and use the 'subtree' merge strategy, -instead of treating the other project as a submodule. Directories -that come from both projects can be cloned and checked out as a whole -if you choose to go that route. +A Submodule allows you to keep another Git repository in a subdirectory +of your repository. The other repository has its own history, which does not +interfere with the history of the current repository. This can be used to +have external dependencies such as third party libraries for example. + +When cloning or pulling a repository containing submodules however, +these will not be checked out by default; the 'init' and 'update' +subcommands will maintain submodules checked out and at +appropriate revision in your working tree. Submodules are composed from a so-called `gitlink` tree entry in the main repository that refers to a particular commit object @@ -51,19 +46,18 @@ describes the default URL the submodule shall be cloned from. The logical name can be used for overriding this URL within your local repository configuration (see 'submodule init'). -This command will manage the tree entries and contents of the -gitmodules file for you, as well as inspect the status of your -submodules and update them. -When adding a new submodule to the tree, the 'add' subcommand -is to be used. However, when pulling a tree containing submodules, -these will not be checked out by default; -the 'init' and 'update' subcommands will maintain submodules -checked out and at appropriate revision in your working tree. -You can briefly inspect the up-to-date status of your submodules -using the 'status' subcommand and get a detailed overview of the -difference between the index and checkouts using the 'summary' -subcommand. - +Submodules are not to be confused with remotes, which are other +repositories of the same project; submodules are meant for +different projects you would like to make part of your source tree, +while the history of the two projects still stays completely +independent and you cannot modify the contents of the submodule +from within the main project. +If you want to merge the project histories and want to treat the +aggregated whole as a single project from then on, you may want to +add a remote for the other project and use the 'subtree' merge strategy, +instead of treating the other project as a submodule. Directories +that come from both projects can be cloned and checked out as a whole +if you choose to go that route. COMMANDS -- 2.4.1.345.gab207b6.dirty -- To unsubscribe from thi
Re: [PATCH/RFC 1/2] git-rebase -i: Add key word "drop" to remove a commit
Junio C Hamano writes: > Matthieu Moy writes: > >> I find it weird to write >> >> noop > > True, but then it can be spelled > > # I do find it weird too. "#" means "comment", which means "do as if it was not there" to me. And in this case it does change the semantics once you activate the safety feature: error out without the "# ", rebase dropping the commit if the comment is present. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 3/3] clone: add `--seed` shorthand
Jeff King writes: > On Sun, May 24, 2015 at 12:07:53PM -0700, Junio C Hamano wrote: > >> Jeff King writes: >> >> > Having slept on it, I really think "--seed" should be "fetch from the >> > seed into temp refs", and not what I posted earlier. >> >> Yeah, I think that is the right way to do it. > > In the meantime, do you want to pick up patches 1 and 2? I think they > are cleanups that stand on their own, whether we do patch 3 or not. Thanks for reminding. Let me take a look. Thanks. -- 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 1/2] git-rebase -i: Add key word "drop" to remove a commit
Matthieu Moy writes: > I find it weird to write > > noop True, but then it can be spelled # too, so do we still want 'drop'? Unless we have a strong reason to believe migrants from Hg cannot be (re)trained, personally, I'd feel that we do not need this 'drop' thing. -- 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 2/2] git rebase -i: Warn removed or dupplicated commits
Matthieu Moy writes: > Stephen Kelly writes: > >> Galan Rémi ensimag.grenoble-inp.fr> writes: >> >>> >>> Check if commits were removed (i.e. a line was deleted) or dupplicated >>> (e.g. the same commit is picked twice), can print warnings or abort >>> git rebase according to the value of the configuration variable >>> rebase.checkLevel. >> >> I sometimes duplicate commits deliberately if I want to split a commit in >> two. I move a copy up and fix the conflict, and I know that I'll still get >> the right thing later even if I make a mistake with the conflict >> resolution. > > The more I think about it, the more I think we should either not warn at > all on duplicate commits, or have a separate config variable. Yeah, I'd say we shouldn't warn, without configuration to keep things simple. > > It's rare to duplicate by mistake, and when you do so, it's already easy > to notice: you get conflicts, and you can git rebase --skip the second > occurence. Accidentally dropped commits are another story: it's rather > easy to cut-and-forget-to-paste, and the consequence currently is silent > data loss ... -- 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 2/2] git rebase -i: Warn removed or dupplicated commits
Remi Galan Alfonso
writes:
> Thank you for reviewing the code.
>
> Eric Sunshine writes:
>> > + # To uppercase
>> > + checkLevel=$(echo "$checkLevel" | tr '[:lower:]' '[:upper:]')
>>
>> Is there precedence elsewhere for recognizing uppercase and lowercase
>> variants of config values?
>
> It seems to be commonly used when parsing options in the C files
> through strcasecmp. For exemple, in config.c:818 :
> if (!strcmp(var, "core.safecrlf")) {
> if (value && !strcasecmp(value, "warn")) {
> [...]
> However we didn't see any precedence in shell files. Do you think we
> should remove it?
I think there is a difference between (silently) accepting just to
be lenient and documenting and advocating mixed case uses.
Personally, I'd rather not to see gratuitous flexibility to allow
the same thing spelled in 47 different ways for no good reason.
--
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/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number
On Tue, May 26, 2015 at 11:39 PM, Jeff King wrote:
> On Tue, May 26, 2015 at 03:01:09PM -0700, Stefan Beller wrote:
>
>> + OPT_STRING('y', "transport-version", &transport_version,
>> +N_("transport-version"),
>> +N_("specify transport version to be used")),
>
> Interesting choice for the short option ("-v" would be nice, but
> obviously it is taken). Do we want to delay on claiming the
> short-and-sweet 'y' until we are sure this is something people will use
> a lot? In an ideal world, it is not (i.e., auto-upgrade and other tricks
> become good enough that nobody bothers to specify it manually).
I made the choice this way:
"Oh crap! -v is taken. so which letter is most likely not taken, so I
can move on?"
So if you have any other proposal with actual reasons I'd switch in a
heart beat.
I figured the -y is good to testing and debugging, but in an ideal
world we don't
want people messing with the transport option because of automatic upgrades
as you said.
Or do you rather hint on dropping the short option at all, and just having NULL
in the field?
>
> -Peff
--
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/WIP PATCH 05/11] transport: add infrastructure to support a protocol version number
Jeff King writes:
> On Tue, May 26, 2015 at 03:01:09PM -0700, Stefan Beller wrote:
>
>> +OPT_STRING('y', "transport-version", &transport_version,
>> + N_("transport-version"),
>> + N_("specify transport version to be used")),
>
> Interesting choice for the short option ("-v" would be nice, but
> obviously it is taken). Do we want to delay on claiming the
> short-and-sweet 'y' until we are sure this is something people will use
> a lot? In an ideal world, it is not (i.e., auto-upgrade and other tricks
> become good enough that nobody bothers to specify it manually).
Yes, just stuff 0 (not NULL but NUL) there; unless we have a very
good reason to believe that the option will be used every day to
toggle per invocation settings, we shouldn't squat on a short and
sweet single letter.
--
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/WIP 1/8] wrapper: implement xopen()
On 2015-05-27 15.33, Paul Tan wrote:
> A common usage pattern of open() is to check if it was successful, and
> die() if it was not:
>
> int fd = open(path, O_WRONLY | O_CREAT, 0777);
> if (fd < 0)
> die_errno(_("Could not open '%s' for writing."), path);
>
> Implement a wrapper function xopen() that does the above so that we can
> save a few lines of code, and make the die() messages consistent.
>
> Signed-off-by: Paul Tan
> ---
> git-compat-util.h | 1 +
> wrapper.c | 18 ++
> 2 files changed, 19 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 17584ad..9745962 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
> extern void *xrealloc(void *ptr, size_t size);
> extern void *xcalloc(size_t nmemb, size_t size);
> extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
> off_t offset);
> +extern int xopen(const char *path, int flags, mode_t mode);
> extern ssize_t xread(int fd, void *buf, size_t len);
> extern ssize_t xwrite(int fd, const void *buf, size_t len);
> extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
> diff --git a/wrapper.c b/wrapper.c
> index c1a663f..971665a 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -189,6 +189,24 @@ void *xcalloc(size_t nmemb, size_t size)
> # endif
> #endif
>
The original open can take 2 or 3 parameters, how about this:
int xopen(const char *path, int oflag, ... )
{
va_list params;
int mode;
int fd;
va_start(params, oflag);
mode = va_arg(params, int);
va_end(params);
fd = open(path, oflag, mode);
> +/**
> + * xopen() is the same as open(), but it die()s if the open() fails.
> + */
> +int xopen(const char *path, int flags, mode_t mode)
> +{
> + int fd;
> +
> + assert(path);
> + fd = open(path, flags, mode);
> + if (fd < 0) {
> + if ((flags & O_WRONLY) || (flags & O_RDWR))
> + die_errno(_("could not open '%s' for writing"), path);
This is only partly true:
it could be either "writing" or "read write".
I don't know if the info "for reading" or "for writing" is needed/helpful at
all,
or if a simple "could not open" would be enough.
Another thing:
should we handle EINTR ?
(Somewhere in the back of my head I remember that some OS
returned EINTR when handling some foreign file system
Mac OS / NTFS ?)
--
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/4] showing existing ws breakage
Jeff King writes: > In color.diff.*, these are called "new", "old", and "plain". I am of the > opinion that "context" is a far better name than "plain", but perhaps we > should support both for consistency. > > Here's a patch for the color.diff side, if we want to go that route. > > -- >8 -- > Subject: diff: accept color.diff.context as a synonym for "plain" > > The term "plain" is a bit ambiguous; let's allow the more > specific "context", but keep "plain" around for > compatibility. > > Signed-off-by: Jeff King > --- > I didn't bother mentioning the historical "plain" in the documentation. > I don't know if it's better to (for people who find it in the wild and > wonder what it means) or if it simply clutters the description. 'plain' does sound a misnomer, as these slot names are about "what" are painted, not "how" they are painted. The latter is what their values represent. Whoever named that slot was confused by the fact that 'context' (i.e. "what") lines are by default painted in 'plain' color without frills (i.e. "how"). We usually try to give a brief mention to historical names primarily to silence those who pick up stale information from the Web, get curious, and then complain loudly after finding that we no longer document them even though we keep accepting them silently, so I am somewhat tempted to do this on top. diff --git a/Documentation/config.txt b/Documentation/config.txt index 0a7ffa5..b458590 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -870,7 +870,8 @@ command line with the `--color[=]` option. color.diff.:: Use customized color for diff colorization. `` specifies which part of the patch to use the specified color, and is one - of `context` (context text), `meta` (metainformation), `frag` + of `context` (context text - `plain` is a historical synonym), + `meta` (metainformation), `frag` (hunk header), 'func' (function in hunk header), `old` (removed lines), `new` (added lines), `commit` (commit headers), or `whitespace` (highlighting whitespace errors). The values of these variables may be -- 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/WIP 1/8] wrapper: implement xopen()
On Wed, May 27, 2015 at 6:33 AM, Paul Tan wrote:
> A common usage pattern of open() is to check if it was successful, and
> die() if it was not:
>
> int fd = open(path, O_WRONLY | O_CREAT, 0777);
> if (fd < 0)
> die_errno(_("Could not open '%s' for writing."), path);
>
> Implement a wrapper function xopen() that does the above so that we can
> save a few lines of code, and make the die() messages consistent.
As a mental todo note for whomever wants to pick up some work: This patch
series indicates to only touch git-am. The first 2 patches introduce
new x-wrappers,
so maybe we'd need to grep through the whole code base and convert the all
these file opening code to the new wrapper.
>
> Signed-off-by: Paul Tan
> ---
> git-compat-util.h | 1 +
> wrapper.c | 18 ++
> 2 files changed, 19 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 17584ad..9745962 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
> extern void *xrealloc(void *ptr, size_t size);
> extern void *xcalloc(size_t nmemb, size_t size);
> extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
> off_t offset);
> +extern int xopen(const char *path, int flags, mode_t mode);
> extern ssize_t xread(int fd, void *buf, size_t len);
> extern ssize_t xwrite(int fd, const void *buf, size_t len);
> extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
> diff --git a/wrapper.c b/wrapper.c
> index c1a663f..971665a 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -189,6 +189,24 @@ void *xcalloc(size_t nmemb, size_t size)
> # endif
> #endif
>
> +/**
> + * xopen() is the same as open(), but it die()s if the open() fails.
> + */
> +int xopen(const char *path, int flags, mode_t mode)
> +{
> + int fd;
> +
> + assert(path);
> + fd = open(path, flags, mode);
> + if (fd < 0) {
> + if ((flags & O_WRONLY) || (flags & O_RDWR))
> + die_errno(_("could not open '%s' for writing"), path);
> + else
> + die_errno(_("could not open '%s' for reading"), path);
> + }
> + return fd;
> +}
> +
> /*
> * xread() is the same a read(), but it automatically restarts read()
> * operations with a recoverable error (EAGAIN and EINTR). xread()
> --
> 2.1.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/RFC 2/2] git rebase -i: Warn removed or dupplicated commits
On Wed, May 27, 2015 at 9:19 AM, Remi Galan Alfonso
wrote:
> Eric Sunshine writes:
>> > + # To uppercase
>> > + checkLevel=$(echo "$checkLevel" | tr '[:lower:]' '[:upper:]')
>>
>> Is there precedence elsewhere for recognizing uppercase and lowercase
>> variants of config values?
>
> It seems to be commonly used when parsing options in the C files
> through strcasecmp. For exemple, in config.c:818 :
> if (!strcmp(var, "core.safecrlf")) {
> if (value && !strcasecmp(value, "warn")) {
> [...]
> However we didn't see any precedence in shell files. Do you think we
> should remove it?
Precedence in C code is good enough for me, and it makes sense for
your new code to follow suit by being insensitive to case (as you have
already done).
However, it would be a good idea to be consistent in your use of
uppercase/lowercase in the commit message, documentation, and code,
rather than having a mix. I'd suggest sticking with lowercase
throughout since lowercase is more commonly used in the codebase (and
just easier to read).
--
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/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Tue, May 26, 2015 at 11:35 PM, Jeff King wrote:
> On Tue, May 26, 2015 at 03:01:08PM -0700, Stefan Beller wrote:
>
>> --- a/upload-pack.c
>> +++ b/upload-pack.c
>> @@ -716,10 +716,47 @@ static void format_symref_info(struct strbuf *buf,
>> struct string_list *symref)
>> strbuf_addf(buf, " symref=%s:%s", item->string, (char
>> *)item->util);
>> }
>>
>> -static const char *advertise_capabilities = "multi_ack thin-pack side-band"
>> +static char *advertise_capabilities = "multi_ack thin-pack side-band"
>> " side-band-64k ofs-delta shallow no-progress"
>> " include-tag multi_ack_detailed";
>
> If we are upload-pack-2, should we advertise that in the capabilities? I
> think it may make things easier later if we try to provide some
> opportunistic out-of-band data. E.g., if see tell git-daemon:
>
> git-upload-pack repo\0host=whatever\0\0version=2
>
> how do we know whether version=2 was understood and kicked us into v2
> mode, versus an old server that ignored it?
So in my vision we would call git-upload-pack-2 instead of having a "version=2".
and if git-upload-pack-2 doesn't exist, the whole conversation is
over, the client
it is left to make up some good error message or retry version 1.
But I think advertising both which versions the server could deal
with, as well as
the currently expected version is a good thing.
capability: can_speak=1,2
capability: speaking_now=2
>
> It also just makes the protocol more self-describing; from the
> conversation you can see which version is in use.
>
>> +static void send_capabilities(void)
>> +{
>> + char buf[100];
>> +
>> + while (next_capability(buf))
>> + packet_write(1, "capability:%s\n", buf);
>
> Having a static-sized buffer and then passing it to a function which has
> no idea of the buffer size seems like a recipe for a buffer overflow. :)
Yes. All patches I proposed are very brittle. My first goal was to have the
last test passing (an actual fetch with version 2). Now I will start looking at
making things robust, as by now you all seem to not disagree totally.
>
> You are fine here because you are parsing the hard-coded capabilities
> string, and we know 100 is enough there. But it's nice to avoid such
> magic.
>
> Like Eric, I find the whole next_capability thing a little ugly. His
> suggestion to pass in the parsing state is an improvement, but I wonder
> why we need to parse at all. Can we keep the capabilities as:
>
> const char *capabilities[] = {
> "multi_ack",
> "thin-pack",
> ... etc ...
> };
>
> and then loop over the array? The v1 writer will need to be modified,
> but it should be fairly straightforward (loop and add them to the buffer
> rather than dumping the whole string).
Thanks for the design guidance! I'll change that.
>
> Also, do we need the capability noise-word?
I thought it opens up a new possible door in the future.
As we ignore anything not starting with "capability" for now, you
could introduce
your foo and bar ping pong easily and still be version 2 compatible.
S: capability: thin
S: capability: another-capability
S: ping-pong foo
S: done
C: (not having understood ping-pong) just answering with capability: thin
C: done, let's proceed to refs advertisement
The alternative client would do:
C: ping-pong: foo-data1a
S: ping-pong: foo-data1b
C: ping-pong: foo-data2a
C: capability: thin
...
> They all have it, except
> for:
>
>> + packet_write(1, "agent:%s\n", git_user_agent_sanitized());
>
> But isn't that basically a capability, too (I agree it is stretching the
> definition of "capability", but I think that ship has sailed; the
> client's response is not "I support this, too" but "I want to enable
> this").
>
> IOW, is there a reason that the initial conversation is not just:
>
> pkt-line("multi_ack");
> pkt-line("thin-pack");
> ...
> pkt-line("agent=v2.5.0");
> pkt-line();
>
> We already have framing for each capability due to the use of pkt-line.
> The "capability:" is just one more thing the client has to parse past.
> The keys are already unique up to any "=", so I don't think there is any
> ambiguity (e.g., we don't care about "capability:agent"; we have already
> assigned a meaning to the term "agent", and will never introduce a
> standalone capability with the same name).
It looks clearer to me (we're not wasting band-width), maybe this ping pong
thing can be part of the capabilities announcement too, so we're good this way.
>
> Likewise, if we introduce new protocol items here, the client should
> either ignore them (if it does not understand them) or know what to do
> with them.
>
>> +static void receive_capabilities(void)
>> +{
>> + int done = 0;
>> + while (1) {
>> + char *line = packet_read_line(0, NULL);
>> + if (!line)
>> + break;
>> + if (starts_with(line, "capability:"))
>> + parse_features(line + strlen("capabilit
Re: [RFC/WIP PATCH 04/11] upload-pack-2: Implement the version 2 of upload-pack
On Wed, May 27, 2015 at 2:35 AM, Jeff King wrote:
> On Tue, May 26, 2015 at 03:01:08PM -0700, Stefan Beller wrote:
>> +static void send_capabilities(void)
>> +{
>> + char buf[100];
>> +
>> + while (next_capability(buf))
>> + packet_write(1, "capability:%s\n", buf);
>
> Like Eric, I find the whole next_capability thing a little ugly. His
> suggestion to pass in the parsing state is an improvement, but I wonder
> why we need to parse at all. Can we keep the capabilities as:
>
> const char *capabilities[] = {
> "multi_ack",
> "thin-pack",
> ... etc ...
> };
>
> and then loop over the array?
Yes, that would be much nicer. I also had this in mind but didn't know
if there was a strong reason for the capabilities to be shoehorned
into a single string as they are currently.
--
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/WIP PATCH 06/11] remote.h: add get_remote_capabilities, request_capabilities
On Wed, May 27, 2015 at 2:50 AM, Jeff King wrote: > On Tue, May 26, 2015 at 11:25:05PM -0400, Eric Sunshine wrote: > >> > + len = packet_read(in, &src_buf, &src_len, >> > + packet_buffer, sizeof(packet_buffer), >> > + PACKET_READ_GENTLE_ON_EOF | >> > + PACKET_READ_CHOMP_NEWLINE); >> > + if (len < 0) >> > + die_initial_contact(0); >> > + >> > + if (!len) >> > + break; >> > + >> > + if (len > 4 && skip_prefix(line, "ERR ", &arg)) >> >> The 'len > 4' check is needed because there's no guarantee that 'line' >> is NUL-terminated. Correct? > > I think this was just blindly copied from get_remote_heads(). And I > think that code was being overly paranoid. Ever since f3a3214 (Make > send/receive-pack be closer to doing something interesting, 2005-06-29), > the pkt-line reader will add an extra NUL to the buffer to ease cases > like this. Thanks. I had started digging into packet_read() to determine whether it guaranteed NUL-termination, but didn't get far enough to decide. I agree that if NUL-termination is guaranteed, then the 'len > 4' check is superfluous (and confusing, which is why it caught my attention in the first place). -- 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
Important Notification
You are required to click on the link to verify your email account because we are upgrading our webmail.http://www.skywap.ro/logo/eupdate/ Webmail Technical Support Copyright 2012. All Rights Reserved -- 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 1/2] git-rebase -i: Add key word "drop" to remove a commit
Remi Galan Alfonso writes: > It also has some effects with the second part of this patch (checks > removed and/or duplicated commits): if you comment the line, the > commit will be considered as removed, thus ending in a warning if the > config variable is set to warn/error; however this problem won't > appear with noop. Indeed, that's the whole point of having a "drop" command. As an advice for your next submission: use "git send-email --cover-letter", and explain the overall idea before the patches. I personally prefer "drop" to "noop" as a command name: I understand "noop" as a command without argument (useful to say "this is actually an empty list of commands, not an empty file to ask rebase to abort"), but I find it weird to write noop As Remi wrote, the inspiration comes from Mercurial. Perhaps we should ask on the mercurial ml how happy they are with the name. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 1/2] git-rebase -i: Add key word "drop" to remove a commit
Thank you for reviewing the code. Johannes Schindelin writes: > Please note that you can already just comment-out the line if you need to > keep a visual trace. > > Alternatively, you can replace the `pick` command by `noop`. > > If you really need the `drop` command (with which I am not 100% > happy because I already envisage users appending a `drop A` to an > edit script "pick A; pick B; pick C" and expecting A *not to be > picked*) It is true that drop has the same effect as noop or commenting, however we thought that drop is more understandable for average users of git. Moreover when using git rebase -i, the 'help' displayed below the list of commits doesn't mention neither the noop command nor the effect of commenting the line (though considering what removing a line does, it can be easily deduced). The drop command was inspired by the drop command from histedit in mercurial. It also has some effects with the second part of this patch (checks removed and/or duplicated commits): if you comment the line, the commit will be considered as removed, thus ending in a warning if the config variable is set to warn/error; however this problem won't appear with noop. -- 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/WIP 3/8] am: implement patch queue mechanism
git-am applies a series of patches. If the process terminates
abnormally, we want to be able to resume applying the series of patches.
This requires the session state to be saved in a persistent location.
Implement the mechanism of a "patch queue", represented by 2 integers --
the index of the current patch we are applying and the index of the last
patch, as well as its lifecycle through the following functions:
* am_setup(), which will set up the state directory
$GIT_DIR/rebase-apply. As such, even if the process exits abnormally,
the last-known state will still persist.
* am_state_load(), which is called if there is an am session in
progress, to load the last known state from the state directory so we
can resume applying patches.
* am_run(), which will do the actual patch application. After applying a
patch, it calls am_next() to increment the current patch index. The
logic for applying and committing a patch is not implemented yet.
* am_destroy(), which is finally called when we successfully applied all
the patches in the queue, to clean up by removing the state directory
and its contents.
Signed-off-by: Paul Tan
---
Makefile | 2 +-
builtin.h| 1 +
builtin/am.c | 167 +++
git.c| 1 +
4 files changed, 170 insertions(+), 1 deletion(-)
create mode 100644 builtin/am.c
diff --git a/Makefile b/Makefile
index 323c401..57a7c8c 100644
--- a/Makefile
+++ b/Makefile
@@ -466,7 +466,6 @@ TEST_PROGRAMS_NEED_X =
# interactive shell sessions without exporting it.
unexport CDPATH
-SCRIPT_SH += git-am.sh
SCRIPT_SH += git-bisect.sh
SCRIPT_SH += git-difftool--helper.sh
SCRIPT_SH += git-filter-branch.sh
@@ -812,6 +811,7 @@ LIB_OBJS += xdiff-interface.o
LIB_OBJS += zlib.o
BUILTIN_OBJS += builtin/add.o
+BUILTIN_OBJS += builtin/am.o
BUILTIN_OBJS += builtin/annotate.o
BUILTIN_OBJS += builtin/apply.o
BUILTIN_OBJS += builtin/archive.o
diff --git a/builtin.h b/builtin.h
index b87df70..d50c9d1 100644
--- a/builtin.h
+++ b/builtin.h
@@ -30,6 +30,7 @@ extern int textconv_object(const char *path, unsigned mode,
const unsigned char
extern int is_builtin(const char *s);
extern int cmd_add(int argc, const char **argv, const char *prefix);
+extern int cmd_am(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix);
extern int cmd_archive(int argc, const char **argv, const char *prefix);
diff --git a/builtin/am.c b/builtin/am.c
new file mode 100644
index 000..6c9
--- /dev/null
+++ b/builtin/am.c
@@ -0,0 +1,167 @@
+/*
+ * Builtin "git am"
+ *
+ * Based on git-am.sh by Junio C Hamano.
+ */
+#include "cache.h"
+#include "parse-options.h"
+#include "dir.h"
+
+struct am_state {
+ struct strbuf dir;/* state directory path */
+ int cur; /* current patch number */
+ int last; /* last patch number */
+};
+
+/**
+ * Initializes am_state with the default values.
+ */
+static void am_state_init(struct am_state *state)
+{
+ memset(state, 0, sizeof(*state));
+
+ strbuf_init(&state->dir, 0);
+}
+
+/**
+ * Release memory allocated by an am_state.
+ */
+static void am_state_release(struct am_state *state)
+{
+ strbuf_release(&state->dir);
+}
+
+/**
+ * Returns path relative to the am_state directory.
+ */
+static inline const char *am_path(const struct am_state *state, const char
*path)
+{
+ return mkpath("%s/%s", state->dir.buf, path);
+}
+
+/**
+ * Returns 1 if there is an am session in progress, 0 otherwise.
+ */
+static int am_in_progress(const struct am_state *state)
+{
+ struct stat st;
+
+ if (lstat(state->dir.buf, &st) < 0 || !S_ISDIR(st.st_mode))
+ return 0;
+ if (lstat(am_path(state, "last"), &st) || !S_ISREG(st.st_mode))
+ return 0;
+ if (lstat(am_path(state, "next"), &st) || !S_ISREG(st.st_mode))
+ return 0;
+ return 1;
+}
+
+/**
+ * Reads the contents of `file`. The third argument can be used to give a hint
+ * about the file size, to avoid reallocs. Returns 0 on success, -1 if the file
+ * does not exist.
+ */
+static int read_state_file(struct strbuf *sb, const char *file, size_t hint) {
+ strbuf_reset(sb);
+
+ if (!strbuf_read_file(sb, file, hint))
+ return 0;
+
+ if (errno == ENOENT)
+ return -1;
+
+ die_errno(_("could not read '%s'"), file);
+}
+
+/**
+ * Loads state from disk.
+ */
+static void am_state_load(struct am_state *state)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ read_state_file(&sb, am_path(state, "next"), 8);
+ state->cur = strtol(sb.buf, NULL, 10);
+
+ read_state_file(&sb, am_path(state, "last"), 8);
+ state->last = strtol(sb.buf, NULL, 10);
+
+ strbuf_release(&sb);
+}
+
+/**
+ * Remove the am_state director
[PATCH/WIP 4/8] am: split out mbox/maildir patches with git-mailsplit
git-am.sh supports mbox, stgit and mercurial patches. Re-implement
support for splitting out mbox/maildirs using git-mailsplit, while also
implementing the framework required to support other patch formats in
the future.
Re-implement support for the --patch-format option (since a5a6755
(git-am foreign patch support: introduce patch_format, 2009-05-27)) to
allow the user to choose between the different patch formats.
Signed-off-by: Paul Tan
---
builtin/am.c | 104 ---
1 file changed, 100 insertions(+), 4 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 6c9..9c7b058 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -6,11 +6,18 @@
#include "cache.h"
#include "parse-options.h"
#include "dir.h"
+#include "run-command.h"
+
+enum patch_format {
+ PATCH_FORMAT_UNKNOWN = 0,
+ PATCH_FORMAT_MBOX
+};
struct am_state {
struct strbuf dir;/* state directory path */
int cur; /* current patch number */
int last; /* last patch number */
+ int prec; /* number of digits in patch filename */
};
/**
@@ -21,6 +28,7 @@ static void am_state_init(struct am_state *state)
memset(state, 0, sizeof(*state));
strbuf_init(&state->dir, 0);
+ state->prec = 4;
}
/**
@@ -101,13 +109,67 @@ static void am_destroy(const struct am_state *state)
}
/**
+ * Splits out individual patches from `paths`, where each path is either a mbox
+ * file or a Maildir. Return 0 on success, -1 on failure.
+ */
+static int split_patches_mbox(struct am_state *state, struct string_list
*paths)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct string_list_item *item;
+ struct strbuf last = STRBUF_INIT;
+
+ cp.git_cmd = 1;
+ argv_array_push(&cp.args, "mailsplit");
+ argv_array_pushf(&cp.args, "-d%d", state->prec);
+ argv_array_pushf(&cp.args, "-o%s", state->dir.buf);
+ argv_array_push(&cp.args, "-b");
+ argv_array_push(&cp.args, "--");
+
+ for_each_string_list_item(item, paths)
+ argv_array_push(&cp.args, item->string);
+
+ if (capture_command(&cp, &last, 8))
+ return -1;
+
+ state->cur = 1;
+ state->last = strtol(last.buf, NULL, 10);
+
+ return 0;
+}
+
+/**
+ * Splits out individual patches, of patch_format, contained within paths.
+ * These patches will be stored in the state directory, with each patch's
+ * filename being its index, padded to state->prec digits. state->cur will be
+ * set to the index of the first patch, and state->last will be set to the
+ * index of the last patch. Returns 0 on success, -1 on failure.
+ */
+static int split_patches(struct am_state *state, enum patch_format
patch_format,
+ struct string_list *paths)
+{
+ switch (patch_format) {
+ case PATCH_FORMAT_MBOX:
+ return split_patches_mbox(state, paths);
+ default:
+ die("BUG: invalid patch_format");
+ }
+ return -1;
+}
+
+/**
* Setup a new am session for applying patches
*/
-static void am_setup(struct am_state *state)
+static void am_setup(struct am_state *state, enum patch_format patch_format,
+ struct string_list *paths)
{
if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
die_errno(_("failed to create directory '%s'"), state->dir.buf);
+ if (split_patches(state, patch_format, paths) < 0) {
+ am_destroy(state);
+ die(_("Failed to split patches."));
+ }
+
write_file(am_path(state, "next"), 1, "%d", state->cur);
write_file(am_path(state, "last"), 1, "%d", state->last);
@@ -128,13 +190,32 @@ static void am_next(struct am_state *state)
*/
static void am_run(struct am_state *state)
{
- while (state->cur <= state->last)
+ while (state->cur <= state->last) {
+ /* patch application not implemented yet */
+
am_next(state);
+ }
am_destroy(state);
}
+/**
+ * parse_options() callback that validates and sets opt->value to the
+ * PATCH_FORMAT_* enum value corresponding to `arg`.
+ */
+static int parse_opt_patchformat(const struct option *opt, const char *arg,
int unset)
+{
+ int *opt_value = opt->value;
+
+ if (!strcmp(arg, "mbox"))
+ *opt_value = PATCH_FORMAT_MBOX;
+ else
+ return -1;
+ return 0;
+}
+
struct am_state state;
+int opt_patch_format;
static const char * const am_usage[] = {
N_("git am [options] [(|)...]"),
@@ -142,6 +223,8 @@ static const char * const am_usage[] = {
};
static struct option am_options[] = {
+ OPT_CALLBACK(0, "patch-format", &opt_patch_format, N_("format"),
+ N_("format the patch(es) are in"), parse_opt_patchformat),
OPT_END()
};
@@ -156,8 +239,21 @@ in
[PATCH/WIP 7/8] am: apply patch with git-apply
Implement applying the patch to the index using git-apply.
Signed-off-by: Paul Tan
---
builtin/am.c | 50 +-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/builtin/am.c b/builtin/am.c
index 0b8a42d..7126df3 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -25,6 +25,18 @@ static int is_empty_file(const char *filename)
return !st.st_size;
}
+/**
+ * Returns the first line of msg
+ */
+static const char *firstline(const char *msg)
+{
+ static struct strbuf sb = STRBUF_INIT;
+
+ strbuf_reset(&sb);
+ strbuf_add(&sb, msg, strchrnul(msg, '\n') - msg);
+ return sb.buf;
+}
+
enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
PATCH_FORMAT_MBOX
@@ -503,6 +515,29 @@ static int parse_patch(struct am_state *state, const char
*patch)
return 0;
}
+/*
+ * Applies current patch with git-apply. Returns 0 on success, -1 otherwise.
+ */
+static int run_apply(const struct am_state *state)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+
+ cp.git_cmd = 1;
+
+ argv_array_push(&cp.args, "apply");
+ argv_array_push(&cp.args, "--index");
+ argv_array_push(&cp.args, am_path(state, "patch"));
+
+ if (run_command(&cp))
+ return -1;
+
+ /* Reload index as git-apply will have modified it. */
+ discard_cache();
+ read_cache();
+
+ return 0;
+}
+
/**
* Applies all queued patches.
*/
@@ -520,7 +555,20 @@ static void am_run(struct am_state *state)
write_file(am_path(state, "final-commit"), 1, "%s",
state->msg.buf);
write_author_script(state);
- /* patch application not implemented yet */
+ printf_ln(_("Applying: %s"), firstline(state->msg.buf));
+
+ if (run_apply(state) < 0) {
+ int value;
+
+ printf_ln(_("Patch failed at %s %s"), msgnum(state),
+ firstline(state->msg.buf));
+
+ if (!git_config_get_bool("advice.amworkdir", &value) &&
!value)
+ printf_ln(_("The copy of the patch that failed
is found in: %s"),
+ am_path(state, "patch"));
+
+ exit(128);
+ }
next:
am_next(state);
--
2.1.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/WIP 8/8] am: commit applied patch
Implement do_commit(), which commits the index which contains the
results of applying the patch, along with the extracted commit message
and authorship information.
Signed-off-by: Paul Tan
---
builtin/am.c | 46 ++
1 file changed, 46 insertions(+)
diff --git a/builtin/am.c b/builtin/am.c
index 7126df3..3174327 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -8,6 +8,9 @@
#include "dir.h"
#include "run-command.h"
#include "quote.h"
+#include "cache-tree.h"
+#include "refs.h"
+#include "commit.h"
/**
* Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -539,6 +542,48 @@ static int run_apply(const struct am_state *state)
}
/**
+ * Commits the current index with state->msg as the commit message and
+ * state->author_name, state->author_email and state->author_date as the author
+ * information.
+ */
+static void do_commit(const struct am_state *state)
+{
+ unsigned char tree[20], parent[20], commit[20];
+ unsigned char *ptr;
+ struct commit_list *parents = NULL;
+ const char *reflog_msg, *author;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (write_cache_as_tree(tree, 0, NULL))
+ die(_("git write-tree failed to write a tree"));
+
+ if (!get_sha1_commit("HEAD", parent)) {
+ ptr = parent;
+ commit_list_insert(lookup_commit(parent), &parents);
+ } else {
+ ptr = NULL;
+ fprintf_ln(stderr, _("applying to an empty history"));
+ }
+
+ author = fmt_ident(state->author_name.buf, state->author_email.buf,
+ state->author_date.buf, IDENT_STRICT);
+
+ if (commit_tree(state->msg.buf, state->msg.len, tree, parents, commit,
+ author, NULL))
+ die(_("failed to write commit object"));
+
+ reflog_msg = getenv("GIT_REFLOG_ACTION");
+ if (!reflog_msg)
+ reflog_msg = "am";
+
+ strbuf_addf(&sb, "%s: %s", reflog_msg, firstline(state->msg.buf));
+
+ update_ref(sb.buf, "HEAD", commit, ptr, 0, UPDATE_REFS_DIE_ON_ERR);
+
+ strbuf_release(&sb);
+}
+
+/**
* Applies all queued patches.
*/
static void am_run(struct am_state *state)
@@ -570,6 +615,7 @@ static void am_run(struct am_state *state)
exit(128);
}
+ do_commit(state);
next:
am_next(state);
}
--
2.1.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/WIP 5/8] am: detect mbox patches
Since 15ced75 (git-am foreign patch support: autodetect some patch
formats, 2009-05-27), git-am.sh is able to autodetect mbox, stgit and
mercurial patches through heuristics.
Re-implement support for autodetecting mbox/maildir files.
Signed-off-by: Paul Tan
---
builtin/am.c | 99
1 file changed, 99 insertions(+)
diff --git a/builtin/am.c b/builtin/am.c
index 9c7b058..d589ec5 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -108,6 +108,97 @@ static void am_destroy(const struct am_state *state)
strbuf_release(&sb);
}
+/*
+ * Returns 1 if the file looks like a piece of email a-la RFC2822, 0 otherwise.
+ * We check this by grabbing all the non-indented lines and seeing if they look
+ * like they begin with valid header field names.
+ */
+static int is_email(const char *filename)
+{
+ struct strbuf sb = STRBUF_INIT;
+ FILE *fp = xfopen(filename, "r");
+ int ret = 1;
+
+ while (!strbuf_getline(&sb, fp, '\n')) {
+ const char *x;
+
+ strbuf_rtrim(&sb);
+
+ if (!sb.len)
+ break; /* End of header */
+
+ /* Ignore indented folded lines */
+ if (*sb.buf == '\t' || *sb.buf == ' ')
+ continue;
+
+ /* It's a header if it matches the regexp "^[!-9;-~]+:" */
+ for (x = sb.buf; *x; x++) {
+ if (('!' <= *x && *x <= '9') || (';' <= *x && *x <=
'~'))
+ continue;
+ if (*x == ':' && x != sb.buf)
+ break;
+ ret = 0;
+ goto fail;
+ }
+ }
+
+fail:
+ fclose(fp);
+ strbuf_release(&sb);
+ return ret;
+}
+
+/**
+ * Attempts to detect the patch_format of the patches contained in `paths`,
+ * returning the PATCH_FORMAT_* enum value. Returns PATCH_FORMAT_UNKNOWN if
+ * detection fails.
+ */
+static int detect_patch_format(struct string_list *paths)
+{
+ enum patch_format ret = PATCH_FORMAT_UNKNOWN;
+ struct strbuf l1 = STRBUF_INIT;
+ struct strbuf l2 = STRBUF_INIT;
+ struct strbuf l3 = STRBUF_INIT;
+ FILE *fp;
+
+ /*
+* We default to mbox format if input is from stdin and for directories
+*/
+ if (!paths->nr || !strcmp(paths->items->string, "-") ||
+ is_directory(paths->items->string)) {
+ strbuf_release(&l1);
+ strbuf_release(&l2);
+ strbuf_release(&l3);
+ return PATCH_FORMAT_MBOX;
+ }
+
+ /*
+* Otherwise, check the first few 3 lines of the first patch, starting
+* from the first non-blank line, to try to detect its format.
+*/
+ fp = xfopen(paths->items->string, "r");
+ while (!strbuf_getline(&l1, fp, '\n')) {
+ strbuf_trim(&l1);
+ if (l1.len)
+ break;
+ }
+ strbuf_getline(&l2, fp, '\n');
+ strbuf_trim(&l2);
+ strbuf_getline(&l3, fp, '\n');
+ strbuf_trim(&l3);
+ fclose(fp);
+
+ if (starts_with(l1.buf, "From ") || starts_with(l1.buf, "From: "))
+ ret = PATCH_FORMAT_MBOX;
+ else if (l1.len && l2.len && l3.len && is_email(paths->items->string))
+ ret = PATCH_FORMAT_MBOX;
+
+ strbuf_release(&l1);
+ strbuf_release(&l2);
+ strbuf_release(&l3);
+ return ret;
+}
+
/**
* Splits out individual patches from `paths`, where each path is either a mbox
* file or a Maildir. Return 0 on success, -1 on failure.
@@ -162,6 +253,14 @@ static int split_patches(struct am_state *state, enum
patch_format patch_format,
static void am_setup(struct am_state *state, enum patch_format patch_format,
struct string_list *paths)
{
+ if (!patch_format)
+ patch_format = detect_patch_format(paths);
+
+ if (!patch_format) {
+ fprintf_ln(stderr, _("Patch format detection failed."));
+ exit(128);
+ }
+
if (mkdir(state->dir.buf, 0777) < 0 && errno != EEXIST)
die_errno(_("failed to create directory '%s'"), state->dir.buf);
--
2.1.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/WIP 6/8] am: extract patch, message and authorship with git-mailinfo
For the purpose of applying the patch and committing the results,
implement extracting the patch data, commit message and authorship from
an e-mail message using git-mailinfo.
git-mailinfo is run as a separate process, but ideally in the future,
we should be be able to access its functionality directly without
spawning a new process.
Signed-off-by: Paul Tan
---
builtin/am.c | 231 +++
1 file changed, 231 insertions(+)
diff --git a/builtin/am.c b/builtin/am.c
index d589ec5..0b8a42d 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -7,6 +7,23 @@
#include "parse-options.h"
#include "dir.h"
#include "run-command.h"
+#include "quote.h"
+
+/**
+ * Returns 1 if the file is empty or does not exist, 0 otherwise.
+ */
+static int is_empty_file(const char *filename)
+{
+ struct stat st;
+
+ if (stat(filename, &st) < 0) {
+ if (errno == ENOENT)
+ return 1;
+ die_errno(_("could not stat %s"), filename);
+ }
+
+ return !st.st_size;
+}
enum patch_format {
PATCH_FORMAT_UNKNOWN = 0,
@@ -17,6 +34,10 @@ struct am_state {
struct strbuf dir;/* state directory path */
int cur; /* current patch number */
int last; /* last patch number */
+ struct strbuf msg;/* commit message */
+ struct strbuf author_name;/* commit author's name */
+ struct strbuf author_email; /* commit author's email */
+ struct strbuf author_date;/* commit author's date */
int prec; /* number of digits in patch filename */
};
@@ -28,6 +49,10 @@ static void am_state_init(struct am_state *state)
memset(state, 0, sizeof(*state));
strbuf_init(&state->dir, 0);
+ strbuf_init(&state->msg, 0);
+ strbuf_init(&state->author_name, 0);
+ strbuf_init(&state->author_email, 0);
+ strbuf_init(&state->author_date, 0);
state->prec = 4;
}
@@ -37,6 +62,10 @@ static void am_state_init(struct am_state *state)
static void am_state_release(struct am_state *state)
{
strbuf_release(&state->dir);
+ strbuf_release(&state->msg);
+ strbuf_release(&state->author_name);
+ strbuf_release(&state->author_email);
+ strbuf_release(&state->author_date);
}
/**
@@ -81,6 +110,92 @@ static int read_state_file(struct strbuf *sb, const char
*file, size_t hint) {
}
/**
+ * Parses the "author script" `filename`, and sets state->author_name,
+ * state->author_email and state->author_date accordingly. We are strict with
+ * our parsing, as the author script is supposed to be eval'd, and loosely
+ * parsing it may not give the results the user expects.
+ *
+ * The author script is of the format:
+ *
+ * GIT_AUTHOR_NAME='$author_name'
+ * GIT_AUTHOR_EMAIL='$author_email'
+ *GIT_AUTHOR_DATE='$author_date'
+ *
+ * where $author_name, $author_email and $author_date are quoted.
+ */
+static int read_author_script(struct am_state *state)
+{
+ char *value;
+ struct strbuf sb = STRBUF_INIT;
+ const char *filename = am_path(state, "author-script");
+ FILE *fp = fopen(filename, "r");
+ if (!fp) {
+ if (errno == ENOENT)
+ return 0;
+ die(_("could not open '%s' for reading"), filename);
+ }
+
+ if (strbuf_getline(&sb, fp, '\n'))
+ return -1;
+ if (!skip_prefix(sb.buf, "GIT_AUTHOR_NAME=", (const char**) &value))
+ return -1;
+ value = sq_dequote(value);
+ if (!value)
+ return -1;
+ strbuf_addstr(&state->author_name, value);
+
+ if (strbuf_getline(&sb, fp, '\n'))
+ return -1;
+ if (!skip_prefix(sb.buf, "GIT_AUTHOR_EMAIL=", (const char**) &value))
+ return -1;
+ value = sq_dequote(value);
+ if (!value)
+ return -1;
+ strbuf_addstr(&state->author_email, value);
+
+ if (strbuf_getline(&sb, fp, '\n'))
+ return -1;
+ if (!skip_prefix(sb.buf, "GIT_AUTHOR_DATE=", (const char**) &value))
+ return -1;
+ value = sq_dequote(value);
+ if (!value)
+ return -1;
+ strbuf_addstr(&state->author_date, value);
+
+ if (fgetc(fp) != EOF)
+ return -1;
+
+ fclose(fp);
+ strbuf_release(&sb);
+ return 0;
+}
+
+/**
+ * Saves state->author_name, state->author_email and state->author_date in
+ * `filename` as an "author script", which is the format used by git-am.sh.
+ */
+static void write_author_script(const struct am_state *state)
+{
+ static const char fmt[] = "GIT_AUTHOR_NAME='%s'\n"
+ "GIT_AUTHOR_EMAIL='%s'\n"
+ "GIT_AUTHOR_DATE='%s'\n";
+ struct strbuf author_name = STRBUF_INIT;
+ struct strbuf author_email = STRBUF_INIT;
+ struct strbuf author
[PATCH/WIP 1/8] wrapper: implement xopen()
A common usage pattern of open() is to check if it was successful, and
die() if it was not:
int fd = open(path, O_WRONLY | O_CREAT, 0777);
if (fd < 0)
die_errno(_("Could not open '%s' for writing."), path);
Implement a wrapper function xopen() that does the above so that we can
save a few lines of code, and make the die() messages consistent.
Signed-off-by: Paul Tan
---
git-compat-util.h | 1 +
wrapper.c | 18 ++
2 files changed, 19 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 17584ad..9745962 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd,
off_t offset);
+extern int xopen(const char *path, int flags, mode_t mode);
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/wrapper.c b/wrapper.c
index c1a663f..971665a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -189,6 +189,24 @@ void *xcalloc(size_t nmemb, size_t size)
# endif
#endif
+/**
+ * xopen() is the same as open(), but it die()s if the open() fails.
+ */
+int xopen(const char *path, int flags, mode_t mode)
+{
+ int fd;
+
+ assert(path);
+ fd = open(path, flags, mode);
+ if (fd < 0) {
+ if ((flags & O_WRONLY) || (flags & O_RDWR))
+ die_errno(_("could not open '%s' for writing"), path);
+ else
+ die_errno(_("could not open '%s' for reading"), path);
+ }
+ return fd;
+}
+
/*
* xread() is the same a read(), but it automatically restarts read()
* operations with a recoverable error (EAGAIN and EINTR). xread()
--
2.1.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/WIP 2/8] wrapper: implement xfopen()
A common usage pattern of fopen() is to check if it succeeded, and die()
if it failed:
FILE *fp = fopen(path, "w");
if (!fp)
die_errno(_("could not open '%s' for writing"), path);
Implement a wrapper function xfopen() for the above, so that we can save
a few lines of code and make the die() messages consistent.
Signed-off-by: Paul Tan
---
git-compat-util.h | 1 +
wrapper.c | 19 +++
2 files changed, 20 insertions(+)
diff --git a/git-compat-util.h b/git-compat-util.h
index 9745962..914d450 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -723,6 +723,7 @@ extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
extern int xdup(int fd);
+extern FILE *xfopen(const char *path, const char *mode);
extern FILE *xfdopen(int fd, const char *mode);
extern int xmkstemp(char *template);
extern int xmkstemp_mode(char *template, int mode);
diff --git a/wrapper.c b/wrapper.c
index 971665a..d5ed780 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -329,6 +329,25 @@ int xdup(int fd)
return ret;
}
+/**
+ * xfopen() is the same as fopen(), but it die()s if the fopen() fails.
+ */
+FILE *xfopen(const char *path, const char *mode)
+{
+ FILE *fp;
+
+ assert(path);
+ assert(mode);
+ fp = fopen(path, mode);
+ if (!fp) {
+ if (*mode == 'w' || *mode == 'a')
+ die_errno(_("could not open '%s' for writing"), path);
+ else
+ die_errno(_("could not open '%s' for reading"), path);
+ }
+ return fp;
+}
+
FILE *xfdopen(int fd, const char *mode)
{
FILE *stream = fdopen(fd, mode);
--
2.1.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/WIP 0/8] Make git-am a builtin
git-am is a commonly used command for applying a series of patches from a mailbox to the current branch. Currently, it is implemented by the shell script git-am.sh. However, compared to C, shell scripts have certain deficiencies: they need to spawn a lot of processes, introduce a lot of dependencies and cannot take advantage of git's internal caches. This WIP patch series rewrites git-am.sh into optimized C builtin/am.c. It is based on my finished prototype of the rewrite[1], and over the next 5 weeks I will be cutting out small patches from the prototype to make it easier to review and refine the patch series. This is part of my GSoC project to rewrite git-pull.sh into git-am.sh into C builtins[2]. A small benchmark that applies 50 patches[3]: #!/bin/sh git init && echo initial >file && git add file && git commit -m initial && git branch before-am && for x in $(seq 50) do echo $x >>file && git commit -a -m $x done && git format-patch --stdout before-am.. >patches && git checkout before-am && time git patches >/dev/null I ran this benchmark on my *Linux* system. Timings for git on master: 1.40s, 1.42s, 1.25s, 1.32s, 1.10s. Avg: ~1.30s Timings for git on master + this patch series applied: 0.24s, 0.22s, 0.22s, 0.19s, 0.25s. Avg: ~0.22s This is around a 6x speedup. It's not because this patch series does less than git-am.sh -- similar speedups can be observed with the prototype, which passes the test suite[4]. (Sorry for leaving the other reviews hanging. I was too preoccupied with the git-am rewrite, and was afraid of forgetting important details should I context switch. Now that the prototype is finished I can deal with the other patch series'.) [1] https://github.com/pyokagan/git/compare/master...pyokagan:pt/ref-builtin-am [2] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 [3] Since a 56-patch series was posted recently ;-) [4] The subset of the test suite that calls git-rebase and git-am. Paul Tan (8): wrapper: implement xopen() wrapper: implement xfopen() am: implement patch queue mechanism am: split out mbox/maildir patches with git-mailsplit am: detect mbox patches am: extract patch, message and authorship with git-mailinfo am: apply patch with git-apply am: commit applied patch Makefile | 2 +- builtin.h | 1 + builtin/am.c | 687 ++ git-compat-util.h | 2 + git.c | 1 + wrapper.c | 37 +++ 6 files changed, 729 insertions(+), 1 deletion(-) create mode 100644 builtin/am.c -- 2.1.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
Bug: .gitconfig folder
If you have a folder named ~/.gitconfig instead of a file with that name, when you try to run some global config editing command it will fail with a wrong error message: "fatal: Out of memory? mmap failed: No such device" You can reproduce it: $rm ~/.gitconfig $mkdir ~/.gitconfig $ls -la ~ ... drwxr-xr-x 24 hit hit 4096 may 4 12:30 .gimp-2.8 drwxr-xr-x 2 hit hit 4096 may 27 15:26 .gitconfig drwxr-xr-x 6 hit hit 4096 may 27 14:01 github ... $git config --global user.name foo fatal: Out of memory? mmap failed: No such device $git config --global core.editor "vim" fatal: Out of memory? mmap failed: No such device -- 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 2/2] git rebase -i: Warn removed or dupplicated commits
Eric Sunshine writes: > Shouldn't this case also 'die' when rebase.checkLevel is "error"? And, > why doesn't the user get advice about configuring rebase.checkLevel in > this case? Stephen Kelly writes: > I sometimes duplicate commits deliberately if I want to split a commit in > two. Matthieu Moy writes: > The more I think about it, the more I think we should either not warn at > all on duplicate commits, or have a separate config variable. Put in common because two config variables would have an effect on the 'die' and advise part. In this patch we didn't put the 'die' in the duplicate commit part since there was only one config variable and there are cases where the user might want to duplicate commits. After the code reviewing of Eric Sunshine and Stephen Kelly, we also came to the conclusion that we should use two config variables, one about missing commits and the other about duplicate commits. This way if you deliberately want to use duplicate commits, you can just set the value to 'ignore' for duplicate commits and still have 'warn'/'error' for missing commits. Moreover, each part would have its 'die' depending on the value of the corresponding config variable. -- 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
FW: Query on git submodules
Thanks Heiko for getting back to me. Correct when I referred to 10+ layers I meant nested repositories which make up a large hierarchy. Some repositories are repeated across the hierarchy. We check-out submodules to tag versions (as opposed to master branch). If we need to roll out a particular change across a hierarchy (e.g. 60 repos) then every level in the hierarchy needs to pick up new submodule tags. The main 2 issues that we see are: 1. Enforcement of absolute paths in git submodules - I am currently trialing using a pre-commit hook to highlight this issue to users so that they can fix their submodule urls to relative paths. 2. Slowness Integrating updates to submodule hierarchy -I have been looking at ways of automating such a roll out - this can be useful for new project setups or for rolling out an update to all repos and quickly integrating it into the submodule hierarchy. The link below shows something similar however it checks out a master branch of each submodule in its hierarchy. https://chrisjean.com/recursively-updating-git-submodules/ Thanks, Sarah -Original Message- From: Heiko Voigt [mailto:hvo...@hvoigt.net] Sent: Tuesday, May 26, 2015 6:01 PM To: Frawley, Sarah Cc: git@vger.kernel.org Subject: Re: Query on git submodules Hi, On Fri, May 22, 2015 at 01:46:24PM +, Frawley, Sarah wrote: > I am a design automation engineer supporting 200+ designers who use > git for hardware design. We also use the submodule feature where we > can have quite complex hierarchy's with 10+ layers. What does this 10+ layers mean? Nested submodule repositories 10 recursion steps deep? > We have experience issues with re-use of design projects was we move >from one derivative to another due to the complexity of the hierarchy >along with lack of discipline (using absolute paths in .gitmodule >files). To enforce more discipline I am currently working on a >pre-commit hook to check the integrity of .gitmodule files. I would be >interested in seeing how other users in the community find submodules >for large scale projects and if there are any best known methods for >using them. I do not have anything to share here since our projects did not have such problems (not that large scale). It would be interesting to see what you come up with. Maybe we can add some of that into core git to support such large scale projects better. So maybe you can share exactly what problems you have (only absolute paths?) or the pre-commit hooks you will use. Cheers Heiko - Intel Ireland Limited (Branch) Collinstown Industrial Park, Leixlip, County Kildare, Ireland Registered Number: E902934 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- 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 2/2] git rebase -i: Warn removed or dupplicated commits
Thank you for reviewing the code.
Eric Sunshine writes:
> > + # To uppercase
> > + checkLevel=$(echo "$checkLevel" | tr '[:lower:]' '[:upper:]')
>
> Is there precedence elsewhere for recognizing uppercase and lowercase
> variants of config values?
It seems to be commonly used when parsing options in the C files
through strcasecmp. For exemple, in config.c:818 :
if (!strcmp(var, "core.safecrlf")) {
if (value && !strcasecmp(value, "warn")) {
[...]
However we didn't see any precedence in shell files. Do you think we
should remove it?
--
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 verify_lock() to report errors via strbuf
On 05/23/2015 01:34 AM, Michael Haggerty wrote: > verify_lock() is a helper function called while committing reference > transactions. But when it fails, instead of recording its error > message in a strbuf to be passed back to the caller of > ref_transaction_commit(), the error message was being written directly > to stderr. > > Instead, report the errors via a strbuf so that they make it back to > the caller of ref_transaction_commit(). > > [...] > > This is the patch series that I mentioned here [1]. It applies on top > of mh/ref-directory-file [2] and is thematically a continuation of > that series in the sense that it further cleans up the error handling > within reference transactions. It would be easy to rebase to master if > that is preferred. The last sentence is nonsense. This patch series relies on lock_ref_sha1_basic() having a "strbuf *err" parameter, which is only the case since 4a32b2e lock_ref_sha1_basic(): report errors via a "struct strbuf *err" (2015-05-11) The latter commit is in mh/ref-directory-file (which has now been merged to master, so technically the last sentence is now correct again). Sorry for the confusion. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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 2/2] git rebase -i: Warn removed or dupplicated commits
Stephen Kelly writes: > Galan Rémi ensimag.grenoble-inp.fr> writes: > >> >> Check if commits were removed (i.e. a line was deleted) or dupplicated >> (e.g. the same commit is picked twice), can print warnings or abort >> git rebase according to the value of the configuration variable >> rebase.checkLevel. > > I sometimes duplicate commits deliberately if I want to split a commit in > two. I move a copy up and fix the conflict, and I know that I'll still get > the right thing later even if I make a mistake with the conflict > resolution. The more I think about it, the more I think we should either not warn at all on duplicate commits, or have a separate config variable. It's rare to duplicate by mistake, and when you do so, it's already easy to notice: you get conflicts, and you can git rebase --skip the second occurence. Accidentally dropped commits are another story: it's rather easy to cut-and-forget-to-paste, and the consequence currently is silent data loss ... -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
Recovering from 'fatal: core.bare and core.worktree do not make sense'
Hi, the other day I said 'git config core.worktree /somewhere' in a bare repo while thinking I was in a regular one, user error. The 'fatal: core.bare and core.worktree do not make sense' error from the next command made me realize immediately that I was wrong, that's good. However... OK, let's have a look and recover from the situation: $ git config --edit fatal: core.bare and core.worktree do not make sense Well, all was well before I set 'core.worktree', so let's unset it: $ git config --unset core.worktree fatal: core.bare and core.worktree do not make sense Hmph, not expecting much, but how about unsetting the other variable? $ git config --unset core.bare fatal: core.bare and core.worktree do not make sense Good, at least it's pretty consistent, though I still don't get what 'git config' has to do with the worktree that is so important that it has to bail out. Time to look for help: $ git help config fatal: core.bare and core.worktree do not make sense WTF :) Alright, I give up: $ vim config $ # happy It was two days later that I had a bit of a lightbulb moment, reproduced the situation and just for fun tried this: $ git -c core.bare=false config --unset core.bare I didn't expect, but it worked! Great. Some thoughts: 1) Perhaps 'git config' should be more careful in the first place and refuse to set 'core.worktree' when 'core.bare' is already true and vice versa. 2) The damage was done with 'git config', so I expected that I can repair it with "plain" 'git config' (i.e. without 'git -c') as well. 'git config' has nothing to do with the path to the worktree after all. And 'git config --edit' should work regardless of the mess that might be in the config file. 3) 'git help ' should always work, shouldn't it? (Though that's the easiest to remedy, just cd out of the repo, or fire up a new terminal window.) Gábor -- 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] t7063: hide stderr from setup inside prereq
When t7063 starts, it runs "update-index --untracked-cache"
to see if we support the untracked cache. Its output goes
straight to stderr, even if the test is not run with "-v".
Let's wrap it in a prereq that will hide the output by
default, but show it with "-v".
Signed-off-by: Jeff King
---
I noticed this messing up my "prove" output. And it always runs first
with "prove --state=slow", because it has a whopping 17 seconds of
sleeps in it.
t/t7063-status-untracked-cache.sh | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/t/t7063-status-untracked-cache.sh
b/t/t7063-status-untracked-cache.sh
index 2b2ffd7..bd4806c 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -8,10 +8,14 @@ avoid_racy() {
sleep 1
}
-git update-index --untracked-cache
# It's fine if git update-index returns an error code other than one,
# it'll be caught in the first test.
-if test $? -eq 1; then
+test_lazy_prereq UNTRACKED_CACHE '
+ { git update-index --untracked-cache; ret=$?; } &&
+ test $ret -ne 1
+'
+
+if ! test_have_prereq UNTRACKED_CACHE; then
skip_all='This system does not support untracked cache'
test_done
fi
--
2.4.1.552.g6de66a4
--
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 2/2] git rebase -i: Warn removed or dupplicated commits
Galan Rémi ensimag.grenoble-inp.fr> writes: > > Check if commits were removed (i.e. a line was deleted) or dupplicated > (e.g. the same commit is picked twice), can print warnings or abort > git rebase according to the value of the configuration variable > rebase.checkLevel. I sometimes duplicate commits deliberately if I want to split a commit in two. I move a copy up and fix the conflict, and I know that I'll still get the right thing later even if I make a mistake with the conflict resolution.
Re: [PATCH 3/3] clone: add `--seed` shorthand
On Sun, May 24, 2015 at 12:07:53PM -0700, Junio C Hamano wrote: > Jeff King writes: > > > Having slept on it, I really think "--seed" should be "fetch from the > > seed into temp refs", and not what I posted earlier. > > Yeah, I think that is the right way to do it. In the meantime, do you want to pick up patches 1 and 2? I think they are cleanups that stand on their own, whether we do patch 3 or not. -Peff -- 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] git-new-workdir: add windows compatibility
Hi Daniel, On 2015-05-26 19:16, Daniel Smith wrote: > Thanks to everyone for reviewing my proposed patch an providing valuable > feedback. This was my first patch submission to a large open source project > like Git and the whole process was a little daunting. Heh, yeah, it can be quite overwhelming (and this mailing list is very "male", too). Sorry! I hope, though, that it was obvious we all only wanted to be helpful? > On Tue, May 26, 2015 at 9:48 AM, Karsten Blees > wrote: > >> AFAICT, the MSys2 symlink() implementation is pretty smart to detect these >> conditions and fall back to deep copy (aka 'cp -a') if symlinks are not >> supported. >> >> IOW, using 'ln -s' will hopefully "just work" in the upcoming Git for >> Windows 2, thus trying to fix it for MSys1 / Git for Windows 1.9x is >> probably a lost cause. >> > > In that case, I'll abandon this patch and wait for Git for Windows 2 to > come out. Speaking of which: Could you try `git-new-workdir` with Git for Windows 2.x (developers' preview)? https://git-for-windows.github.io/#download It *might* be necessary to define the `MSYS` environment variable to `winsymlinks:nativestrict` before starting the Git Bash, to enable symlinking. It would be really good to know if that is the case so that I can do something about this in the Git for Windows installer. Ciao, Johannes -- 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: Staging commits with visual diff tools?
On Wed, 27 May 2015, Jeff King wrote: If you are interested in looking further, see how the %patch_mode hash is defined in git-add--interactive.perl. Specifically, note that "add -p" is just one case: diff against the index and apply with "apply --cached". But once you have a separate tool for picking hunks, you should be able to drop in the different DIFF/APPLY pairs to implement "checkout -p", etc. My plan was to use symlinks, checkout-index, and update-index --index-info, similar to how difftool does it. I verified on the command line that that seems to work. John -- 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: Staging commits with visual diff tools?
On Tue, May 26, 2015 at 09:50:49PM +0100, John Lee wrote: > Does anybody have code to stage commits using a the visual diff/merge tools > supported by git-difftool? Is there support in git itself somewhere, even? > > I'm looking for something functionally similar to git add -p > > Looking at the git-difftool source I can see how to write a command to do > it, but wanted to check if it had already been implemented. > > Did I miss a way that already exists? git-gui has a visual staging tool, and I think some other git interfaces do, too (e.g., tig). But I don't think there is a way to interact with an arbitrary 3rd-party diff tool. I would think doing so would depend on the diff tool itself; what facility does it provide for picking lines or a hunk out of a diff and communicating it back to git to stage? If you are interested in looking further, see how the %patch_mode hash is defined in git-add--interactive.perl. Specifically, note that "add -p" is just one case: diff against the index and apply with "apply --cached". But once you have a separate tool for picking hunks, you should be able to drop in the different DIFF/APPLY pairs to implement "checkout -p", etc. -Peff -- 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/4] showing existing ws breakage
On Tue, May 26, 2015 at 11:30:28PM -0700, Junio C Hamano wrote:
> The fourth one in v2 used a new option "--[no-]ws-check-deleted",
> but in this round a new option "--ws-error-highlight=" is
> defined instead. With that, you can say
>
> diff --ws-error-highlight=new,old
>
> to say "I want to see whitespace errors on new and old lines", and
>
> diff --ws-error-highlight=new,old,context
> diff --ws-error-highlight=all
>
> can be used to also see whitespace errors on context lines. Being
> able to see whitespace errors on context lines, i.e. the ones that
> were there in the original and you left intact, would help you see
> how prevalent whitespace errors are in the original and hopefully
> would make it easier for you to decide if a separate preliminary
> clean-up to only fix these whitespace errors is warranted.
That makes sense. Neat feature.
In color.diff.*, these are called "new", "old", and "plain". I am of the
opinion that "context" is a far better name than "plain", but perhaps we
should support both for consistency.
Here's a patch for the color.diff side, if we want to go that route.
-- >8 --
Subject: diff: accept color.diff.context as a synonym for "plain"
The term "plain" is a bit ambiguous; let's allow the more
specific "context", but keep "plain" around for
compatibility.
Signed-off-by: Jeff King
---
I didn't bother mentioning the historical "plain" in the documentation.
I don't know if it's better to (for people who find it in the wild and
wonder what it means) or if it simply clutters the description. It may
also be that people don't find "plain" as meaningless as I do, and would
rather leave it as the primary in the documentation (and accepting
"context" is just a nicety).
I also resisted the urge to refactor every internal variable and enum
mentioning "plain" into "context". I guess whether that is a good idea
depends on how strongly you agree that "plain" is a bad name. :)
Documentation/config.txt | 2 +-
diff.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f76e8c..34ef9fe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -914,7 +914,7 @@ command line with the `--color[=]` option.
color.diff.::
Use customized color for diff colorization. `` specifies
which part of the patch to use the specified color, and is one
- of `plain` (context text), `meta` (metainformation), `frag`
+ of `context` (context text), `meta` (metainformation), `frag`
(hunk header), 'func' (function in hunk header), `old` (removed lines),
`new` (added lines), `commit` (commit headers), or `whitespace`
(highlighting whitespace errors).
diff --git a/diff.c b/diff.c
index 7500c55..27bd371 100644
--- a/diff.c
+++ b/diff.c
@@ -54,7 +54,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
static int parse_diff_color_slot(const char *var)
{
- if (!strcasecmp(var, "plain"))
+ if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
return DIFF_PLAIN;
if (!strcasecmp(var, "meta"))
return DIFF_METAINFO;
--
2.4.1.552.g6de66a4
--
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/WIP PATCH 10/11] t5544: add a test case for the new protocol
On Tue, May 26, 2015 at 03:01:14PM -0700, Stefan Beller wrote: > Signed-off-by: Stefan Beller > --- > t/t5544-fetch-2.sh | 40 > 1 file changed, 40 insertions(+) > create mode 100755 t/t5544-fetch-2.sh Obviously we are not there yet, but a fun test will be to globally bump the transport version to "2" and try to run the test suite. We will also want to test interoperation between v1 and v2. Though the _best_ test of that is not hitting a v2 server configured for v1, but hitting an actual older version of git. Which is outside the scope of the current test suite, as it always operates on a single version. It might be nice to have a separate test suite for doing interoperability, that knows how to build various versions (there's already some prior art in t/perf). I know this isn't the first time this concept has come up. -Peff -- 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/WIP PATCH 00/11] Protocol version 2, again!
On Wed, May 27, 2015 at 02:18:18AM -0400, Jeff King wrote: > > The new protocol works just like the old protocol, except for > > the capabilities negotiation being before any exchange of real data. > > I like this approach. [...] So now I've read through all the patches. I still like it. :) There's a lot of work to be done still, but I think this is a great start. Thanks for getting the ball rolling. -Peff -- 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/WIP PATCH 09/11] transport: get_refs_via_connect exchanges capabilities before refs.
On Tue, May 26, 2015 at 03:01:13PM -0700, Stefan Beller wrote:
> + switch (version) {
> + default: /*
> + * Configured a protocol version > 2?
> + * Try version 2 as it's the most future proof.
> + */
> + /* fall through */
Same comment here as earlier. If we do a v3, it might not be compatible
with v2. Shouldn't we bail if the user asked for it?
> + case 2: /* first talk about capabilities, then get the heads */
> + get_remote_capabilities(data->fd[0], NULL, 0);
> + request_capabilities(data->fd[1]);
> + get_remote_heads(data->fd[0], NULL, 0, &refs,
> + for_push ? REF_NORMAL : 0,
> + &data->extra_have,
> + &data->shallow);
> + break;
> + case 1: /* configured version 1, fall through */
> + case 0: /* unconfigured, use first protocol */
> + get_remote_heads(data->fd[0], NULL, 0, &refs,
> + for_push ? REF_NORMAL : 0,
> + &data->extra_have,
> + &data->shallow);
> + break;
> + }
I actually kind of wonder if we should just die("BUG") here on seeing
"0". Is this low level the right place to do the "default to v1"?
Because eventually we're going to want to default to v2, I would think
(after a few years have passed, at least). It seems like we should be
making that decision centrally when we init the transport options.
-Peff
--
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/WIP PATCH 08/11] transport: connect_setup appends protocol version number
On Tue, May 26, 2015 at 03:01:12PM -0700, Stefan Beller wrote:
> Signed-off-by: Stefan Beller
> ---
> transport.c | 18 --
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index 3ef15f6..33644a6 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -496,15 +496,29 @@ static int set_git_option(struct git_transport_options
> *opts,
> static int connect_setup(struct transport *transport, int for_push, int
> verbose)
> {
> struct git_transport_data *data = transport->data;
> + const char *remote_program;
> + char *buf = 0;
Use NULL when you mean a NULL pointer (they're equivalent to the
compiler, but the word is easier to read).
I agree on Eric's naming this "to_free" (and I consider it idiomatic to
assign them in a chain, like "foo = to_free = xmalloc(...)", but we
don't always do that).
> + if (transport->smart_options
> + && transport->smart_options->transport_version) {
> + buf = xmalloc(strlen(remote_program) + 12);
> + sprintf(buf, "%s-%d", remote_program,
> + transport->smart_options->transport_version);
> + remote_program = buf;
> + }
Using xstrfmt can help you avoid magic numbers and repetition,
like:
to_free = xstrfmt("%s-%d",
remote_program,
transport->smart_options->transport_version);
-Peff
--
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
