* RE: will git provide `submodule remove` option ?
From: Lingcha X @ 2013-02-07 5:45 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git@vger.kernel.org
In-Reply-To: <5112CAD4.8060801@web.de>
Hi Jens,
`git rm -r submodule` will not do the same thing, i do NOT know if `git submodule deinit`, i will have some tests.
----
Thanks and Best Regards,
lingchax
----------------------------------------
> Date: Wed, 6 Feb 2013 22:27:48 +0100
> From: Jens.Lehmann@web.de
> To: douglarek@outlook.com
> CC: git@vger.kernel.org
> Subject: Re: will git provide `submodule remove` option ?
>
> Am 05.02.2013 11:32, schrieb Lingcha X:
> > As we all know, git provides `submodule add , init, update, sync, sumary, foreach, status`, but where is `submodule remove`?
> >
> > will
> > I not delete one of them sometime in the future? Although most people
> > will not use submodule or one who uses it can remove submodule by hand, providing complete options may be a good idea.
>
> Is assume either "git rm <submodule>" (available since 1.8.1) or the upcoming
> "git submodule deinit" (currently in pu) will do what you want?
^ permalink raw reply
* Hyper links of london sale occurs in several designs
From: deishei001 @ 2013-02-07 6:34 UTC (permalink / raw)
To: git
The actual style cues tend to be obtained from numerous designs that were a
sizable accomplishment after which blend in to that one piece of incredible
vintage jewellery. Hyper <strong> links of london sale
<http://www.cheapcheaplinksoflondonbracelets.co.uk/> </strong> occurs in
several designs as well as styles because of its huge number of customers.
They've numerous colours as well as designs way too. Therefore apart from
becoming the evidence of companionship, they are able to cause you to
research excellent as well as fashionable. You should use these phones
personalize your own seems. You may also existing these phones your own
buddy, sweetheart, sweetheart, child, boy along with other people. This
really is some thing which would pull a person interest. The actual style
cues tend to be from numerous styles that have been the uniform after which
it blend in to that certain piece of amazing vintage jewellery. This kind of
jewellery includes titanium, precious metal (10 in order to fourteen karat),
surgery metal, as well as niobium is really suggested. The very best element
regarding niobium in addition to titanium is actually these people never
have the response using the chemical substance make-up of our own personal
pores and skin. Nevertheless, surgery metal usually have a slight volume of
nickel however it does not result in a good hypersensitivity. These types of
alloys are often consequently utilized seeing that hypoallergenic jewellery
layer. Platinum eagle as well is produce this sort of jewellery.
Additionally with this, real copper, free of nickel or even nickel
additional alloys, is really a hypoallergenic things with regard to
jewellery.
The actual jewellery number of <strong> links of london friendship bracelet
<http://www.cheapcheaplinksoflondonbracelets.co.uk/> </strong> is actually
remarkable and could complement any kind of modern jewellery shop from any
location. Probably the most desired jewellery included in this may be the
Back links operating out of birmingham Companionship Bracelets. This is
often a wonderful method to help to make brand new buddies or even continue
aged companionship. Among the rest of the number of jewelries the actual
companionship music group is definitely an unique item. It’s awesome as well
as fashionable along with a touch of style as well as guarantee. It's
targeted mostly from teens as well as youngsters obtainable who wish to keep
a signal associated with companionship together. Companionship bracelet
perform behave as the driver within reviving your own companionship. The
actual Hyperlinks operating out of <strong> links of london bracelets
<http://www.cheapcheaplinksoflondonbracelets.co.uk/> </strong> will come in
just about all sizes and shapes, with regard to each man as well as woman.
They're fashionable as well as smooth, supplying personal design claims.
Miniatures from the accurate bracelet will also be accessible right now.
Each and every bracelets occurs within appreciation colors as well as
dimensions to complement a person. To distinguish a few of the couple of
companionship bracelet tend to be small young man, small woman, azure, metal
salt precious metal as well as gray companionship bracelet, darkish
yellow-colored fluoric, darkish lemon, graduation crimson, pewter whitened,
patriotic companionship bracelet and so on.
They've distinctive colours as well as designs as well. Therefore apart from
as being a evidence of companionship, they are able to lead you to appear
awesome as well as fashionable. You need to use these to personalize your
own appears. You may also present these to your own pal, sweetheart,
sweetheart, child, boy while others. Miniatures from the real bracelet will
also be available now. Pretty much every bracelets will come in appreciation
colours as well as dimensions to complement a person. To name a few of the
handful of companionship bracelet tend to be small young man, small woman,
azure, metal salt precious metal as well as off white companionship
bracelet, darkish yellow-colored fluoric, darkish lemon, graduation crimson,
pewter whitened, patriotic <strong> cheap links of london bracelets
<http://www.cheapcheaplinksoflondonbracelets.co.uk/> </strong> and so
forth. Hyperlinks operating out of birmingham Companionship Bracelets is
available in several styles as well as styles because of its wide selection
of purchasers.
http://www.cheapcheaplinksoflondonbracelets.co.uk/
--
View this message in context: http://git.661346.n2.nabble.com/Hyper-links-of-london-sale-occurs-in-several-designs-tp7577205.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] git-send-email: add ~/.authinfo parsing
From: Matthieu Moy @ 2013-02-07 7:08 UTC (permalink / raw)
To: Ted Zlatanov
Cc: Jeff King, Junio C Hamano, Michal Nazarewicz, git,
Krzysztof Mazur, Michal Nazarewicz
In-Reply-To: <8738x9ghpu.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> Logically they are different steps (query and response), even though the
> data protocol is the same. But it's really not a big deal, I know what
> it means either way.
Yes, but if you rename write() to query(), then on the helper side,
you'll have to call query() to send the response, and response() to read
the query. Much worse than keeping read/write.
Plus, read/write has already been used for a while in the C API, so I'd
rather keep the same names for the Perl equivalent.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 3/4] Git.pm: Add interface for git credential command.
From: Matthieu Moy @ 2013-02-07 7:19 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: Junio C Hamano, Ted Zlatanov, Jeff King, git
In-Reply-To: <f4b5413b0a55474346daa7b0866c7a4fed55778d.1360183427.git.mina86@mina86.com>
Michal Nazarewicz <mpn@google.com> writes:
> From: Michal Nazarewicz <mina86@mina86.com>
>
> Add a credential() function which is an interface to the
> git credential command.
Nice.
I think you should credit git-remote-mediawiki for the code in the
commit message. Perhaps have a first "copy/paste" commit, and then an
"adaptation" commit to add sort, ^ anchor in regexp, doc and your
callback mechanism, but I won't insist on that.
Other than that, it all looks good, thanks. I'll take care of deleting
the old code in git-remote-mediawiki and use Git.pm instead.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 3/4] Git.pm: Add interface for git credential command.
From: Matthieu Moy @ 2013-02-07 7:22 UTC (permalink / raw)
To: Michal Nazarewicz; +Cc: git
In-Reply-To: <f4b5413b0a55474346daa7b0866c7a4fed55778d.1360183427.git.mina86@mina86.com>
Michal Nazarewicz <mpn@google.com> writes:
> Subject: [PATCH 3/4] Git.pm: Add interface for git credential command.
Ah, just a nitpick: usually we write the message without capital after
":" and without the final ".".
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH 2/2] count-objects: report garbage files in .git/objects/pack directory too
From: Duy Nguyen @ 2013-02-07 7:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1ucw2bd4.fsf@alter.siamese.dyndns.org>
On Mon, Feb 04, 2013 at 10:16:23AM -0800, Junio C Hamano wrote:
> I forgot to mention one more thing. Your report_pack_garbage()
> special cases ".pack" to see if it is a regular file, but this loop
> structure causes a regular file whose name ends with ".pack" but
> without corresponding ".idx" file to go unreported.
>
> I think the loop should be restructured to iterate over all known
> file types and report unknown ones, if you want to repurpose it for
> the reporting, something along this line, perhaps:
>
> for (each name) {
> if (does it end with ".idx") {
> if (is it unusable ".idx") {
> report garbage;
> }
> continue;
> }
> if (! we are in report mode)
> continue;
> if (does it end with ".pack") {
> if (!have we seen corresponding ".idx")
> remember it;
> continue;
> }
> report garbage;
> }
> for (remembered pack) {
> if (does it have corresponding ".idx" &&
> is it really usable ".pack")
> continue;
> report garbage;
> }
>
How about the below patch? Ignoring good .commits files will be just a
couple more lines in the "for (remembered_pack)" loop.
Also in another mail in this thread:
> I also wonder, especially because you are enumerating the temporary
> pack files in .git/objects/pack/, if it make more sense to account
> for their sizes as well. After all, you would end up doing stat()
> for a common case of files with ".pack" suffix---doing so for all of
> them shouldn't be too bad.
I thought about that, but we may need to do extra stat() for loose
garbage as well. As it is now, garbage is complained loudly, which
gives me enough motivation to clean up, even without looking at how
much disk space it uses.
-- 8< --
Subject: count-objects: report garbage pack directory too
prepare_packed_git_one() is modified to allow count-objects to hook a
report function to so we don't need to duplicate the pack searching
logic in count-objects.c. When report_pack_garbage is NULL, the
overhead is insignificant.
diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
prune-packable: the number of loose objects that are also present in
the packs. These objects could be pruned using `git prune-packed`.
+
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
GIT
---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..7fdd508 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,8 @@
#include "builtin.h"
#include "parse-options.h"
+static unsigned long garbage;
+
static void count_objects(DIR *d, char *path, int len, int verbose,
unsigned long *loose,
off_t *loose_size,
@@ -65,6 +67,16 @@ static void count_objects(DIR *d, char *path, int len, int verbose,
}
}
+extern void (*report_pack_garbage)(const char *path, int len, const char *name);
+static void real_report_pack_garbage(const char *path, int len, const char *name)
+{
+ if (len)
+ error("garbage found: %.*s/%s", len, path, name);
+ else
+ error("garbage found: %s", path);
+ garbage++;
+}
+
static char const * const count_objects_usage[] = {
N_("git count-objects [-v]"),
NULL
@@ -76,7 +88,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
- unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+ unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -87,6 +99,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+ if (verbose)
+ report_pack_garbage = real_report_pack_garbage;
memcpy(path, objdir, len);
if (len && objdir[len-1] != '/')
path[len++] = '/';
diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..5b70e55 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -21,6 +21,7 @@
#include "sha1-lookup.h"
#include "bulk-checkin.h"
#include "streaming.h"
+#include "dir.h"
#ifndef O_NOATIME
#if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
@@ -1000,15 +1001,19 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
}
+void (*report_pack_garbage)(const char *path, int len, const char *name);
+
static void prepare_packed_git_one(char *objdir, int local)
{
/* Ensure that this buffer is large enough so that we can
append "/pack/" without clobbering the stack even if
strlen(objdir) were PATH_MAX. */
char path[PATH_MAX + 1 + 4 + 1 + 1];
- int len;
+ int i, len;
DIR *dir;
struct dirent *de;
+ struct packed_git *p;
+ struct string_list garbage = STRING_LIST_INIT_DUP;
sprintf(path, "%s/pack", objdir);
len = strlen(path);
@@ -1024,14 +1029,33 @@ static void prepare_packed_git_one(char *objdir, int local)
int namelen = strlen(de->d_name);
struct packed_git *p;
- if (!has_extension(de->d_name, ".idx"))
+ if (len + namelen + 1 > sizeof(path)) {
+ if (report_pack_garbage)
+ report_pack_garbage(path, len - 1, de->d_name);
continue;
+ }
- if (len + namelen + 1 > sizeof(path))
+ strcpy(path + len, de->d_name);
+
+ if (!has_extension(de->d_name, ".idx")) {
+ if (!report_pack_garbage)
+ continue;
+ if (is_dot_or_dotdot(de->d_name))
+ continue;
+ if (!has_extension(de->d_name, ".pack")) {
+ report_pack_garbage(path, 0, NULL);
+ continue;
+ }
+ /*
+ * we can't decide right know if this .pack is
+ * garbage. Delay until we identify all good
+ * packs.
+ */
+ string_list_append(&garbage, path);
continue;
+ }
/* Don't reopen a pack we already have. */
- strcpy(path + len, de->d_name);
for (p = packed_git; p; p = p->next) {
if (!memcmp(path, p->pack_name, len + namelen - 4))
break;
@@ -1047,6 +1071,25 @@ static void prepare_packed_git_one(char *objdir, int local)
install_packed_git(p);
}
closedir(dir);
+
+ if (!report_pack_garbage)
+ return;
+
+ sort_string_list(&garbage);
+ for (p = packed_git; p; p = p->next) {
+ struct string_list_item *item;
+ if (!p->pack_local)
+ continue;
+ item = string_list_lookup(&garbage, p->pack_name);
+ if (item)
+ item->util = &garbage; /* anything but NULL */
+ }
+ for (i = 0; i < garbage.nr; i++) {
+ struct string_list_item *item = garbage.items + i;
+ if (!item->util)
+ report_pack_garbage(item->string, 0, NULL);
+ }
+ string_list_clear(&garbage, 0);
}
static int sort_pack(const void *a_, const void *b_)
-- 8< --
^ permalink raw reply related
* Re: [PATCH 2/3] combine-diff: suppress a clang warning
From: John Keeping @ 2013-02-07 8:41 UTC (permalink / raw)
To: Miles Bader; +Cc: Junio C Hamano, git, Antoine Pelisse
In-Reply-To: <876224sqwk.fsf@catnip.gol.com>
On Thu, Feb 07, 2013 at 01:12:59PM +0900, Miles Bader wrote:
> John Keeping <john@keeping.me.uk> writes:
> > I generally like to get rid of the pointless warnings so that the useful
> > ones can't hide in the noise. Perhaps "CFLAGS += -Wno-string-plus-int"
> > would be better for this particular warning, but when there's only one
> > bit of code that triggers it, tweaking that seemed simpler.
>
> An even better approach would be to file a bug against clang ... it
> really is a very ill-considered warning -- PTR + OFFS is not just
> valid C, it's _idiomatic_ in C for getting interior pointers into
> arrays -- and such a warning should never be enabled by default, or by
> any standard warning options.
It doesn't warn of PTR + OFFS, only STRING_LITERAL + OFFS. I agree that
it's not a particularly useful warning but it was clearly introduced
intentionally and appears to find real bugs [1] so I don't intend to
argue about it with the Clang developers.
[1] http://article.gmane.org/gmane.comp.compilers.clang.scm/47203
John
^ permalink raw reply
* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Michael J Gruber @ 2013-02-07 8:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <7vmwvhmli7.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 06.02.2013 17:53:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> Currently, "diff" and "cat-file" for blobs obey "--textconv" options
>> (with the former defaulting to "--textconv" and the latter to
>> "--no-textconv") whereas "show" does not obey this option, even though
>> it takes diff options.
>>
>> Make "show" on blobs behave like "diff", i.e. obey "--textconv" by
>> default and "--no-textconv" when given.
>
> What does "log -p" do currently, and what should it do? Does/should
> it also use --textconv?
It invokes "--textconv" by default, and allows to override with
"--no-textconv. That's what I meant to say but seem to have failed to.
I think at some point we decided to have "human output" on by default
for all things diff (porcelain only, of course) unless the diff is meant
for machine consumption, such as in the case of format-patch.
> The --textconv is a natural extension of what --ext-diff provides us,
> so I think it should trigger the same way as how --ext-diff triggers.
This series' aim is to make the textconv behavior the same for all
"human output" commands. I don't see textconv and ext-diff to be
strongly related, and if then the other way round:
textconv is about converting blobs to a (possibly lossy) human
consumable text.
ext-diff is about computing diffs with an external diff "tool" (not in
the sense of diff-tool).
One way of diffing is textconving blobs then using internal diff on the
resulting text blobs, but ext-diff is more general and, really,
orthogonal in a way. (It used to be used often for what textconv does
when that didn't exist.)
> We apply "--ext-diff" for "diff" by default but not for "log -p" and
> "show"; I suspect this may have been for a good reason but I do not
> recall the discussion that led to the current behaviour offhand.
I don't think I changed anything ext-diff related, did I? 1/4 is about
showing blobs, not diffs.
>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>> ---
>> builtin/log.c | 24 +++++++++++++++++++++---
>> 1 file changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 8f0b2e8..f83870d 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)
>> strbuf_release(&out);
>> }
>>
>> -static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
>> +static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, const char *obj_name)
>> {
>> + unsigned char sha1c[20];
>> + struct object_context obj_context;
>> + char *buf;
>> + unsigned long size;
>> +
>> fflush(stdout);
>> - return stream_blob_to_fd(1, sha1, NULL, 0);
>> + if (!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
>> + return stream_blob_to_fd(1, sha1, NULL, 0);
>> +
>> + if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
>> + die("Not a valid object name %s", obj_name);
>> + if (!obj_context.path[0] ||
>> + !textconv_object(obj_context.path, obj_context.mode, sha1c, 1, &buf, &size))
>> + return stream_blob_to_fd(1, sha1, NULL, 0);
>> +
>> + if (!buf)
>> + die("git show %s: bad file", obj_name);
>> +
>> + write_or_die(1, buf, size);
>> + return 0;
>> }
>>
>> static int show_tag_object(const unsigned char *sha1, struct rev_info *rev)
>> @@ -491,7 +509,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
>> const char *name = objects[i].name;
>> switch (o->type) {
>> case OBJ_BLOB:
>> - ret = show_blob_object(o->sha1, NULL);
>> + ret = show_blob_object(o->sha1, &rev, name);
>> break;
>> case OBJ_TAG: {
>> struct tag *t = (struct tag *)o;
^ permalink raw reply
* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Michael J Gruber @ 2013-02-07 9:05 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20130206220644.GB27507@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 06.02.2013 23:06:
> On Wed, Feb 06, 2013 at 04:08:50PM +0100, Michael J Gruber wrote:
>
>> diff --git a/builtin/log.c b/builtin/log.c
>> index 8f0b2e8..f83870d 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -402,10 +402,28 @@ static void show_tagger(char *buf, int len, struct rev_info *rev)
>> strbuf_release(&out);
>> }
>>
>> -static int show_blob_object(const unsigned char *sha1, struct rev_info *rev)
>> +static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, const char *obj_name)
>
> Should this maybe just take the whole object_array_entry as a cleanup?
It's just a question of one or two/three pointers (I can't count), but
yes, that would be possible.
>> {
>> + unsigned char sha1c[20];
>> + struct object_context obj_context;
>> + char *buf;
>> + unsigned long size;
>> +
>> fflush(stdout);
>> - return stream_blob_to_fd(1, sha1, NULL, 0);
>> + if (!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
>> + return stream_blob_to_fd(1, sha1, NULL, 0);
>> +
>> + if (get_sha1_with_context(obj_name, 0, sha1c, &obj_context))
>> + die("Not a valid object name %s", obj_name);
>
> It seems a little hacky that we have to look up the sha1 again. What
> should happen in the off chance that "hashcmp(sha1, sha1c) != 0" due to
> a race with a simultaneous update of a ref?
I thought about a check here but didn't bother to because I knew the
refactoring would come up again...
> Would it be better if object_array_entry replaced its "mode" member with
> an object_context?
Do all callers/users want to deal with object_context?
I'm wondering why o_c has a mode at all, since it is mostly used in
conjunction with an object, isn't it?
> The only downside I see is that we might waste a
> significant amount of memory (each context has a PATH_MAX buffer in it).
That's why I used a reference to the struct, see my other reply.
Michael
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Michael J Gruber @ 2013-02-07 9:05 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20130206223656.GF27507@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 06.02.2013 23:36:
> On Wed, Feb 06, 2013 at 04:08:53PM +0100, Michael J Gruber wrote:
>
>> - add_object_array(object, arg, &list);
>> + add_object_array_with_context(object, arg, &list, xmemdupz(&oc, sizeof(struct object_context)));
>
> If we go this route, this new _with_context variant should be used in
> patch 1, too.
>
>> @@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
>> objects[nr].item = obj;
>> objects[nr].name = name;
>> objects[nr].mode = mode;
>> + objects[nr].context = context;
>> array->nr = ++nr;
>> }
>
> This seems a little gross. Who is responsible for allocating the
> context? Who frees it? It looks like we duplicate it in cmd_grep. Which
Well, who is responsible for allocating and freeing name and item? I
didn't want to introduce a new member which is a struct when all other
complex members are pointers. Wouldn't that be confusing?
> I think is OK, but it means all of this context infrastructure in
> object.[ch] is just bolted-on junk waiting for somebody to use it wrong
> or get confused. It does not get set, for example, by the regular
> setup_revisions code path.
Sure, it's NULL when there is no context info, just like in many other
cases.
> It would be nice if we could just always have the context available,
> then setup_revisions could set it up by default (and replace the "mode"
> parameter entirely). But we'd need to do something to avoid the
> PATH_MAX-sized buffer for each entry, as some code paths may have a
> large number of pending objects.
If the information is always available even if we don't need it then it
always takes space. The only way out would be pointing into a pool of
path names rather having a copy in each entry. It's not like I hadn't
talked about providing virtual (blob) objects for path names keyed by
their sha1 before... It's just that I want my grep --textconv now ;)
Michael
^ permalink raw reply
* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Jeff King @ 2013-02-07 9:11 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <51136E56.7060703@drmicha.warpmail.net>
On Thu, Feb 07, 2013 at 10:05:26AM +0100, Michael J Gruber wrote:
> > Would it be better if object_array_entry replaced its "mode" member with
> > an object_context?
>
> Do all callers/users want to deal with object_context?
Wouldn't it just mean replacing "entry->mode" with "entry->oc.mode" at
each user?
> I'm wondering why o_c has a mode at all, since it is mostly used in
> conjunction with an object, isn't it?
Just as we record the path from the surrounding tree, we record the
mode. It's that mode which gets put into the pending object list by the
revision parser (see the very end of handle_revision_arg). Storing an
object_context instead of the mode would be a strict superset of what we
store now (right now we just throw the rest away).
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Jeff King @ 2013-02-07 9:26 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <51136E75.5060002@drmicha.warpmail.net>
On Thu, Feb 07, 2013 at 10:05:57AM +0100, Michael J Gruber wrote:
> >> @@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
> >> objects[nr].item = obj;
> >> objects[nr].name = name;
> >> objects[nr].mode = mode;
> >> + objects[nr].context = context;
> >> array->nr = ++nr;
> >> }
> >
> > This seems a little gross. Who is responsible for allocating the
> > context? Who frees it? It looks like we duplicate it in cmd_grep. Which
>
> Well, who is responsible for allocating and freeing name and item? I
> didn't want to introduce a new member which is a struct when all other
> complex members are pointers. Wouldn't that be confusing?
We cheat on those two. "item" is always a pointer to a "struct object",
which lasts forever and never gets freed. When "name" is set by
setup_revisions, it comes from the argv list, which is assumed to last
forever (and when we add pending blobs for a "--objects" traversal, it
is the empty string (literal).
I'd be OK if we had an exterior object_context that could be handled
in the same way. But how do we tell setup_revisions that we are
interested in seeing the object_context from each parsed item, where
does the allocation come from (is it malloc'd by setup_revisions?), and
who is responsible for freeing it when we pop pending objects in
get_revisions and similar?
I don't think it's as clear cut.
I wonder, though...what we really care about here is just the pathname.
But if it is a pending object that comes from a blob revision argument,
won't it always be of the form "treeish:path"? Could we not even resolve
the sha1 again, but instead just parse out the ":path" bit?
That is sort of like what the repeated call to get_sha1_with_context
does in your first patch. Except that we do not actually want to lookup
the sha1, and it is harmful to do so (e.g., if the ref had moved on to a
new tree that does not have that path, get_sha1 would fail, but we do
not even care what is in the tree; we only want the parsing side effects
of get_sha1).
Hmm.
-Peff
PS By the way, while looking at the object_array code (which I have not
really used much before), I noticed that add_pending_commit_list sets
the "name" field to the result of sha1_to_hex. Which means that it is
likely to be completely bogus by the time you read it. I'm not even
sure where it gets read or if this matters. And obviously it's
completely unrelated to what we were discussing; just something I
noticed.
^ permalink raw reply
* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Michael J Gruber @ 2013-02-07 9:34 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20130207091116.GB15727@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 07.02.2013 10:11:
> On Thu, Feb 07, 2013 at 10:05:26AM +0100, Michael J Gruber wrote:
>
>>> Would it be better if object_array_entry replaced its "mode" member with
>>> an object_context?
>>
>> Do all callers/users want to deal with object_context?
>
> Wouldn't it just mean replacing "entry->mode" with "entry->oc.mode" at
> each user?
Yes, I meant at the time of creation, i.e. when someone has to create
and pass an o_a_e and maybe only knows a mode, and thus would have to
set the path to NULL or "".
>> I'm wondering why o_c has a mode at all, since it is mostly used in
>> conjunction with an object, isn't it?
>
> Just as we record the path from the surrounding tree, we record the
> mode. It's that mode which gets put into the pending object list by the
> revision parser (see the very end of handle_revision_arg). Storing an
> object_context instead of the mode would be a strict superset of what we
> store now (right now we just throw the rest away).
Sure. But why does object_context have a mode member at all? Maybe it is
not alway used together with another struct which has the mode already,
then that's a reason.
Michael
^ permalink raw reply
* Re: [PATCH] connect.c: Tell *PLink to always use ssh protocol
From: Sven Strickroth @ 2013-02-07 9:40 UTC (permalink / raw)
To: Jeff King; +Cc: msysgit, git, gitster
In-Reply-To: <20130206232214.GN27507@sigill.intra.peff.net>
Am 07.02.2013 00:22 schrieb Jeff King:
> On Wed, Feb 06, 2013 at 10:58:49PM +0100, Sven Strickroth wrote:
>
>> Default values for *plink can be set using PuTTY. If a user makes
>> telnet the default in PuTTY this breaks ssh clones in git.
>>
>> Since git clones of the type user@host:path use ssh, tell *plink
>> to use ssh and override PuTTY defaults for the protocol to use.
>> ---
>> connect.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/connect.c b/connect.c
>> index 49e56ba..d337b6f 100644
>> --- a/connect.c
>> +++ b/connect.c
>> @@ -625,6 +625,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
>> if (!ssh) ssh = "ssh";
>>
>> *arg++ = ssh;
>> + if (putty)
>> + *arg++ = "-ssh";
>> if (putty && !strcasestr(ssh, "tortoiseplink"))
>> *arg++ = "-batch";
>> if (port) {
Just for the completeness: This might have an unwanted side effect...
Using the -ssh parameter sets the protocol to ssh AND the port number to
22. This might break a setting where a user stores a PuTTY default for
ssh, but with a different port number (e.g. because a user always pushes
to a remote ssh repository which resides on a different port).
PuTTY settings for a named session still work, it only affects the
"Default Settings" session - so users can set up specific sessons in
PuTTY if he wants to change the default port.
--
Best regards,
Sven Strickroth
PGP key id F5A9D4C4 @ any key-server
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [RFC/PATCH 1/4] show: obey --textconv for blobs
From: Jeff King @ 2013-02-07 9:43 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <51137522.3010005@drmicha.warpmail.net>
On Thu, Feb 07, 2013 at 10:34:26AM +0100, Michael J Gruber wrote:
> > Just as we record the path from the surrounding tree, we record the
> > mode. It's that mode which gets put into the pending object list by the
> > revision parser (see the very end of handle_revision_arg). Storing an
> > object_context instead of the mode would be a strict superset of what we
> > store now (right now we just throw the rest away).
>
> Sure. But why does object_context have a mode member at all? Maybe it is
> not alway used together with another struct which has the mode already,
> then that's a reason.
Exactly. It's purely for pulling information out of
get_sha1_with_context, and does not know that you are going to put its
output into an object_array_entry (and many call sites do not).
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Michael J Gruber @ 2013-02-07 9:47 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20130207092640.GC15727@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 07.02.2013 10:26:
> On Thu, Feb 07, 2013 at 10:05:57AM +0100, Michael J Gruber wrote:
>
>>>> @@ -265,9 +260,28 @@ void add_object_array_with_mode(struct object *obj, const char *name, struct obj
>>>> objects[nr].item = obj;
>>>> objects[nr].name = name;
>>>> objects[nr].mode = mode;
>>>> + objects[nr].context = context;
>>>> array->nr = ++nr;
>>>> }
>>>
>>> This seems a little gross. Who is responsible for allocating the
>>> context? Who frees it? It looks like we duplicate it in cmd_grep. Which
>>
>> Well, who is responsible for allocating and freeing name and item? I
>> didn't want to introduce a new member which is a struct when all other
>> complex members are pointers. Wouldn't that be confusing?
>
> We cheat on those two. "item" is always a pointer to a "struct object",
> which lasts forever and never gets freed. When "name" is set by
> setup_revisions, it comes from the argv list, which is assumed to last
> forever (and when we add pending blobs for a "--objects" traversal, it
> is the empty string (literal).
I see, so they are really different.
> I'd be OK if we had an exterior object_context that could be handled
> in the same way. But how do we tell setup_revisions that we are
> interested in seeing the object_context from each parsed item, where
> does the allocation come from (is it malloc'd by setup_revisions?), and
> who is responsible for freeing it when we pop pending objects in
> get_revisions and similar?
Do we really need all of tree, path and mode in object_context (I mean
not just here, but other users), or only the path? I'd try and resurrect
the virtual path name objects then, they would be just like "item"
storage-wise.
> I don't think it's as clear cut.
>
> I wonder, though...what we really care about here is just the pathname.
> But if it is a pending object that comes from a blob revision argument,
> won't it always be of the form "treeish:path"? Could we not even resolve
> the sha1 again, but instead just parse out the ":path" bit?
Do we have that, and in what form (e.g. magic expanded etc.)?
> That is sort of like what the repeated call to get_sha1_with_context
> does in your first patch. Except that we do not actually want to lookup
> the sha1, and it is harmful to do so (e.g., if the ref had moved on to a
> new tree that does not have that path, get_sha1 would fail, but we do
> not even care what is in the tree; we only want the parsing side effects
> of get_sha1).
>
> Hmm.
>
> -Peff
>
> PS By the way, while looking at the object_array code (which I have not
> really used much before), I noticed that add_pending_commit_list sets
> the "name" field to the result of sha1_to_hex. Which means that it is
> likely to be completely bogus by the time you read it. I'm not even
> sure where it gets read or if this matters. And obviously it's
> completely unrelated to what we were discussing; just something I
> noticed.
Another thing I noted is that our path mangling at least for grep has
some issues:
(cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
Taking everything right of ":" could still work.
Michael
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Jeff King @ 2013-02-07 9:55 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <5113784B.10103@drmicha.warpmail.net>
On Thu, Feb 07, 2013 at 10:47:55AM +0100, Michael J Gruber wrote:
> > I'd be OK if we had an exterior object_context that could be handled
> > in the same way. But how do we tell setup_revisions that we are
> > interested in seeing the object_context from each parsed item, where
> > does the allocation come from (is it malloc'd by setup_revisions?), and
> > who is responsible for freeing it when we pop pending objects in
> > get_revisions and similar?
>
> Do we really need all of tree, path and mode in object_context (I mean
> not just here, but other users), or only the path? I'd try and resurrect
> the virtual path name objects then, they would be just like "item"
> storage-wise.
We need at least mode, since that is how the mode parameter of
object_array_entry gets set. I do not know off-hand who uses "tree". I
suspect the intent was to do .gitattributes lookups inside that tree,
but I do not think we actually do in-tree lookups currently.
> > I don't think it's as clear cut.
> >
> > I wonder, though...what we really care about here is just the pathname.
> > But if it is a pending object that comes from a blob revision argument,
> > won't it always be of the form "treeish:path"? Could we not even resolve
> > the sha1 again, but instead just parse out the ":path" bit?
>
> Do we have that, and in what form (e.g. magic expanded etc.)?
Ah, I should have mentioned that. :) We should have the original rev
name in the object_array_entry's name field, shouldn't we? It's just a
matter of re-parsing it.
> Another thing I noted is that our path mangling at least for grep has
> some issues:
>
> (cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
> ../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
Yuck.
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Ævar Arnfjörð Bjarmason @ 2013-02-07 10:30 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Duy Nguyen, Michael Haggerty, Jonathan Nieder,
git, Shawn Pearce
In-Reply-To: <20130207001635.GA29318@sigill.intra.peff.net>
On Thu, Feb 7, 2013 at 1:16 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 06, 2013 at 04:12:10PM -0800, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>> > I think there's a simpler way to do this, which is that:
>> >
>> > * New clients supporting v2 of the protocol send some piece of data
>> > that would break old servers.
>> >
>> > * If that fails the new client goes "oh jeeze, I guess it's an old
>> > server", and try again with the old protocol.
>> >
>> > * The client then saves a date (or the version the server gave us)
>> > indicating that it tried the new protocol on that remote, tries
>> > again sometime later.
>>
>> For that to work, the new server needs to wait for the client to
>> speak first. How would that server handle old clients who expect to
>> be spoken first? Wait with a read timeout (no timeout is the right
>> timeout for everybody)?
>
> If the new client can handle the old-style server's response, then the
> server can start blasting out refs (optionally after a timeout) and stop
> when the client interrupts with "hey, wait, I can speak the new
> protocol". The server just has to include "you can interrupt me" in its
> capability advertisement (obviously it would have to send out at least
> the first ref with the capabilities before the timeout).
Can't this also be handled by passing an extra argument to
upload-pack? Whether you're talking http, ssh + normal shell, ssh +
git-shell or git:// you pass some argument that older clients would
reject on but would cause newer clients that know about that argument
to wait for you to speak before blasting refs at you.
It would mean that older clients (e.g. older git-shell) would reject
your initial connection, but you could just try again, and save away
info about that remote's version.
^ permalink raw reply
* Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path
From: Michael J Gruber @ 2013-02-07 10:31 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20130207095533.GA17110@sigio.peff.net>
Jeff King venit, vidit, dixit 07.02.2013 10:55:
> On Thu, Feb 07, 2013 at 10:47:55AM +0100, Michael J Gruber wrote:
>
>>> I'd be OK if we had an exterior object_context that could be handled
>>> in the same way. But how do we tell setup_revisions that we are
>>> interested in seeing the object_context from each parsed item, where
>>> does the allocation come from (is it malloc'd by setup_revisions?), and
>>> who is responsible for freeing it when we pop pending objects in
>>> get_revisions and similar?
>>
>> Do we really need all of tree, path and mode in object_context (I mean
>> not just here, but other users), or only the path? I'd try and resurrect
>> the virtual path name objects then, they would be just like "item"
>> storage-wise.
>
> We need at least mode, since that is how the mode parameter of
> object_array_entry gets set. I do not know off-hand who uses "tree". I
> suspect the intent was to do .gitattributes lookups inside that tree,
> but I do not think we actually do in-tree lookups currently.
>
>>> I don't think it's as clear cut.
>>>
>>> I wonder, though...what we really care about here is just the pathname.
>>> But if it is a pending object that comes from a blob revision argument,
>>> won't it always be of the form "treeish:path"? Could we not even resolve
>>> the sha1 again, but instead just parse out the ":path" bit?
>>
>> Do we have that, and in what form (e.g. magic expanded etc.)?
>
> Ah, I should have mentioned that. :) We should have the original rev
> name in the object_array_entry's name field, shouldn't we? It's just a
> matter of re-parsing it.
>
>> Another thing I noted is that our path mangling at least for grep has
>> some issues:
>>
>> (cd t && git grep GET_SHA1_QUIETLY HEAD:../cache.h)
>> ../HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
>
> Yuck.
And even more yuck:
(cd t && git grep --full-name GET_SHA1_QUIETLY HEAD:../cache.h)
HEAD:../cache.h:#define GET_SHA1_QUIETLY 01
Someone does not expect a "rev:" to be in there, it seems ;)
Michael
^ permalink raw reply
* [PATCHv2 0/5] Make git-send-email use git-credential
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
From: Michal Nazarewicz <mina86@mina86.com>
Minor fixes as suggested in emails.
Michal Nazarewicz (5):
Git.pm: allow command_close_bidi_pipe to be called as method
Git.pm: fix example in command_close_bidi_pipe documentation
Git.pm: allow pipes to be closed prior to calling
command_close_bidi_pipe
Git.pm: add interface for git credential command
git-send-email: use git credential to obtain password
Documentation/git-send-email.txt | 4 +-
git-send-email.perl | 59 ++++++++++--------
perl/Git.pm | 129 +++++++++++++++++++++++++++++++++++++--
3 files changed, 161 insertions(+), 31 deletions(-)
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply
* [PATCHv2 1/5] Git.pm: allow command_close_bidi_pipe to be called as method
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The documentation of command_close_bidi_pipe() claims that it can
be called as a method, but it does not check whether the first
argument is $self or not assuming the latter. Using _maybe_self()
fixes this.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..bbb753a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -430,7 +430,7 @@ have more complicated structure.
sub command_close_bidi_pipe {
local $?;
- my ($pid, $in, $out, $ctx) = @_;
+ my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
unless (close $fh) {
if ($!) {
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* [PATCHv2 4/5] Git.pm: add interface for git credential command
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
Add a credential() function which is an interface to the git
credential command. The code is heavily based on credential_*
functions in <contrib/mw-to-git/git-remote-mediawiki>.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 109 insertions(+), 1 deletion(-)
On Thu, Feb 07 2013, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 06, 2013 at 09:47:05PM +0100, Michal Nazarewicz wrote:
>
>> +sub _credential_read {
>> + my %credential;
>> + my ($reader, $op) = (@_);
>> + while (<$reader>) {
>> + chomp;
>> + my ($key, $value) = /([^=]*)=(.*)/;
>
> Empty keys are not valid. Can we make this:
>
> /^([^=]+)=(.*)/
>
> to fail the regex? Otherwise, I think this check:
>
>> + if (not defined $key) {
>> + throw Error::Simple("unable to parse git credential $op response:\n$_\n");
>> + }
>
> would not pass because $key would be the empty string.
Right, fixed.
>> +sub _credential_write {
>> + my ($credential, $writer) = @_;
>> +
>> + for my $key (sort {
>> + # url overwrites other fields, so it must come first
>> + return -1 if $a eq 'url';
>> + return 1 if $b eq 'url';
>> + return $a cmp $b;
>> + } keys %$credential) {
>> + if (defined $credential->{$key} && length $credential->{$key}) {
>> + print $writer $key, '=', $credential->{$key}, "\n";
>> + }
>> + }
>
> There are a few disallowed characters, like "\n" in key or value, and
> "=" in a key. They should never happen unless the caller is buggy, but
> should we check and catch them here?
I left it as is for now since it's not entairly clear to me what to
do in all cases. In particular:
- when reading, what to do if the line is " foo = bar ",
- when reading, what to do if the line is "foo=" (ie. empty value),
- when writing, what to do if value is a single space,
- when writing, what to do if value ends with a new line,
- when writing, what to do if value is empty (currently not printed at all),
On Thu, Feb 07 2013, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> I think you should credit git-remote-mediawiki for the code in the
> commit message. Perhaps have a first "copy/paste" commit, and then an
> "adaptation" commit to add sort, ^ anchor in regexp, doc and your
> callback mechanism, but I won't insist on that.
Good point. Creating additional commit is a bit too much for my
licking, but added note in commit message.
diff --git a/perl/Git.pm b/perl/Git.pm
index 9dded54..b4adead 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,7 +59,8 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
- temp_acquire temp_release temp_reset temp_path);
+ temp_acquire temp_release temp_reset temp_path
+ credential);
=head1 DESCRIPTION
@@ -1013,6 +1014,113 @@ sub _close_cat_blob {
}
+sub _credential_read {
+ my %credential;
+ my ($reader, $op) = (@_);
+ while (<$reader>) {
+ if (!/^([^=\s]+)=(.*?)\s*$/) {
+ throw Error::Simple("unable to parse git credential $op response:\n$_");
+ }
+ $credential{$1} = $2;
+ }
+ return %credential;
+}
+
+sub _credential_write {
+ my ($credential, $writer) = @_;
+
+ for my $key (sort {
+ # url overwrites other fields, so it must come first
+ return -1 if $a eq 'url';
+ return 1 if $b eq 'url';
+ return $a cmp $b;
+ } keys %$credential) {
+ if (defined $credential->{$key} && length $credential->{$key}) {
+ print $writer $key, '=', $credential->{$key}, "\n";
+ }
+ }
+ print $writer "\n";
+}
+
+sub _credential_run {
+ my ($self, $credential, $op) = _maybe_self(@_);
+
+ my ($pid, $reader, $writer, $ctx) = command_bidi_pipe('credential', $op);
+
+ _credential_write $credential, $writer;
+ close $writer;
+
+ if ($op eq "fill") {
+ %$credential = _credential_read $reader, $op;
+ } elsif (<$reader>) {
+ throw Error::Simple("unexpected output from git credential $op response:\n$_\n");
+ }
+
+ command_close_bidi_pipe($pid, $reader, undef, $ctx);
+}
+
+=item credential( CREDENTIAL_HASH [, OPERATION ] )
+
+=item credential( CREDENTIAL_HASH, CODE )
+
+Executes C<git credential> for a given set of credentials and
+specified operation. In both form C<CREDENTIAL_HASH> needs to be
+a reference to a hash which stores credentials. Under certain
+conditions the hash can change.
+
+In the first form, C<OPERATION> can be C<'fill'> (or omitted),
+C<'approve'> or C<'reject'>, and function will execute corresponding
+C<git credential> sub-command. In case of C<'fill'> the values stored
+in C<CREDENTIAL_HASH> will be changed to the ones returned by the
+C<git credential> command. The usual usage would look something like:
+
+ my %cred = (
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ );
+ Git::credential \%cred;
+ if (try_to_authenticate($cred{'username'}, $cred{'password'})) {
+ Git::credential \%cred, 'approve';
+ ... do more stuff ...
+ } else {
+ Git::credential \%cred, 'reject';
+ }
+
+In the second form, C<CODE> needs to be a reference to a subroutine.
+The function will execute C<git credential fill> to fill provided
+credential hash, than call C<CODE> with C<CREDENTIAL_HASH> as the sole
+argument, and finally depending on C<CODE>'s return value execute
+C<git credential approve> (if return value yields true) or C<git
+credential reject> (otherwise). The return value is the same as what
+C<CODE> returned. With this form, the usage might look as follows:
+
+ if (Git::credential {
+ 'protocol' => 'https',
+ 'host' => 'example.com',
+ 'username' => 'bob'
+ }, sub {
+ my $cred = shift;
+ return try_to_authenticate($cred->{'username'}, $cred->{'password'});
+ }) {
+ ... do more stuff ...
+ }
+
+=cut
+
+sub credential {
+ my ($self, $credential, $op_or_code) = (_maybe_self(@_), 'fill');
+
+ if ('CODE' eq ref $op_or_code) {
+ _credential_run $credential, 'fill';
+ my $ret = $op_or_code->($credential);
+ _credential_run $credential, $ret ? 'approve' : 'reject';
+ return $ret;
+ } else {
+ _credential_run $credential, $op_or_code;
+ }
+}
+
{ # %TEMP_* Lexical Context
my (%TEMP_FILEMAP, %TEMP_FILES);
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* [PATCHv2 2/5] Git.pm: fix example in command_close_bidi_pipe documentation
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
File handle goes as the first argument when calling print on it.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index bbb753a..11f310a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -418,7 +418,7 @@ and it is the fourth value returned by C<command_bidi_pipe()>. The call idiom
is:
my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
- print "000000000\n" $out;
+ print $out "000000000\n";
while (<$in>) { ... }
$r->command_close_bidi_pipe($pid, $in, $out, $ctx);
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* [PATCHv2 3/5] Git.pm: allow pipes to be closed prior to calling command_close_bidi_pipe
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
The command_close_bidi_pipe() function will insist on closing both
input and output pipes returned by command_bidi_pipe(). With this
change it is possible to close one of the pipes in advance and
pass undef as an argument.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
perl/Git.pm | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
> On Wed, Feb 06, 2013 at 09:47:04PM +0100, Michal Nazarewicz wrote:
>> This allows for something like:
>>
>> my ($pid, $in, $out, $ctx) = command_bidi_pipe(...);
>> print $out "write data";
>> close $out;
>> # ... do stuff with $in
>> command_close_bidi_pipe($pid, $in, undef, $ctx);
On Thu, Feb 07 2013, Jeff King <peff@peff.net> wrote:
> Should this part go into the documentation for command_close_bidi_pipe
> in Git.pm?
Done.
diff --git a/perl/Git.pm b/perl/Git.pm
index 11f310a..9dded54 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -426,13 +426,26 @@ Note that you should not rely on whatever actually is in C<CTX>;
currently it is simply the command name but in future the context might
have more complicated structure.
+C<PIPE_IN> and C<PIPE_OUT> may be C<undef> if they have been closed prior to
+calling this function. This may be useful in a query-response type of
+commands where caller first writes a query and later reads response, eg:
+
+ my ($pid, $in, $out, $ctx) = $r->command_bidi_pipe('cat-file --batch-check');
+ print $out "000000000\n";
+ close $out;
+ while (<$in>) { ... }
+ $r->command_close_bidi_pipe($pid, $in, undef, $ctx);
+
+This idiom may prevent potential dead locks caused by data sent to the output
+pipe not being flushed and thus not reaching the executed command.
+
=cut
sub command_close_bidi_pipe {
local $?;
my ($self, $pid, $in, $out, $ctx) = _maybe_self(@_);
foreach my $fh ($in, $out) {
- unless (close $fh) {
+ if (defined $fh && !close $fh) {
if ($!) {
carp "error closing pipe: $!";
} elsif ($? >> 8) {
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
* [PATCHv2 5/5] git-send-email: use git credential to obtain password
From: Michal Nazarewicz @ 2013-02-07 14:01 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Matthieu Moy; +Cc: git
In-Reply-To: <cover.1360242782.git.mina86@mina86.com>
From: Michal Nazarewicz <mina86@mina86.com>
If smtp_user is provided but smtp_pass is not, instead of
prompting for password, make git-send-email use git
credential command instead.
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
Documentation/git-send-email.txt | 4 +--
git-send-email.perl | 59 +++++++++++++++++++++++-----------------
2 files changed, 36 insertions(+), 27 deletions(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 44a1f7c..0cffef8 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -164,8 +164,8 @@ Sending
Furthermore, passwords need not be specified in configuration files
or on the command line. If a username has been specified (with
'--smtp-user' or a 'sendemail.smtpuser'), but no password has been
-specified (with '--smtp-pass' or 'sendemail.smtppass'), then the
-user is prompted for a password while the input is masked for privacy.
+specified (with '--smtp-pass' or 'sendemail.smtppass'), then
+a password is obtained using 'git-credential'.
--smtp-server=<host>::
If set, specifies the outgoing SMTP server to use (e.g.
diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..76bbfc3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1045,6 +1045,39 @@ sub maildomain {
return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
}
+# Returns 1 if authentication succeeded or was not necessary
+# (smtp_user was not specified), and 0 otherwise.
+
+sub smtp_auth_maybe {
+ if (!defined $smtp_authuser || $auth) {
+ return 1;
+ }
+
+ # Workaround AUTH PLAIN/LOGIN interaction defect
+ # with Authen::SASL::Cyrus
+ eval {
+ require Authen::SASL;
+ Authen::SASL->import(qw(Perl));
+ };
+
+ # TODO: Authentication may fail not because credentials were
+ # invalid but due to other reasons, in which we should not
+ # reject credentials.
+ $auth = Git::credential({
+ 'protocol' => 'smtp',
+ 'host' => join(':', $smtp_server, $smtp_server_port),
+ 'username' => $smtp_authuser,
+ # if there's no password, "git credential fill" will
+ # give us one, otherwise it'll just pass this one.
+ 'password' => $smtp_authpass
+ }, sub {
+ my $cred = shift;
+ return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
+ });
+
+ return $auth;
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1185,31 +1218,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ? " port=$smtp_server_port" : "";
}
- if (defined $smtp_authuser) {
- # Workaround AUTH PLAIN/LOGIN interaction defect
- # with Authen::SASL::Cyrus
- eval {
- require Authen::SASL;
- Authen::SASL->import(qw(Perl));
- };
-
- if (!defined $smtp_authpass) {
-
- system "stty -echo";
-
- do {
- print "Password: ";
- $_ = <STDIN>;
- print "\n";
- } while (!defined $_);
-
- chomp($smtp_authpass = $_);
-
- system "stty echo";
- }
-
- $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
- }
+ smtp_auth_maybe or die $smtp->message;
$smtp->mail( $raw_from ) or die $smtp->message;
$smtp->to( @recipients ) or die $smtp->message;
--
1.8.1.2.549.g1d13f9f
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox