Re: importing two different trees into a fresh git repo

2013-02-06 Thread Jeff King
On Tue, Feb 05, 2013 at 01:46:09PM -0800, Constantine A. Murenin wrote:

 I've encountered two problems so far:
 
 0. After initialising the repository, I was unable to `git checkout
 --orphan Debian-6.0.4-nginx-1.0.12` -- presumably it doesn't work when
 the repo is empty?  This sounds like a bug or an artefact of
 implementation.  I presume this can be worked around by committing
 into master instead, and then doing `git checkout -b
 Debian-6.0.4-nginx-1.0.12`, and then force-fixing the master somehow
 later on.

What version of git are you using? Using both -b and --orphan from a
non-existing branch used to be broken, but was fixed by abe1998 (git
checkout -b: allow switching out of an unborn branch, 2012-01-30), which
first appeared in git v1.7.9.2.

 1. After making a mistake on my first commit (my first commit into
 OpenBSD-5.2-nginx-1.2.2 orphan branch ended up including a directory
 from master by mistake), I am now unable to rebase and fixup the
 changes -- `git rebase --interactive HEAD~2` doesn't work, which, from
 one perspective, makes perfect sense (indeed there's no prior
 revision), but, from another, it's not immediately obvious how to
 quickly work around it.

You cannot ask to rebase onto HEAD~2 because it does not exist (I'm assuming
from your description that HEAD~1 is the root of your repository). But
you can use the --root flag to ask git to rebase all the way down to
the roots, like:

  git rebase -i --root

However, note that older versions of git do not support using --root
with -i. The first usable version is v1.7.12.

-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: git-svn problems with white-space in tag names

2013-02-06 Thread Erik Faye-Lund
On Sun, Jan 27, 2013 at 3:12 PM, Hans-Juergen Euler waas.n...@gmail.com wrote:
 This seems to be a problem of the windows version. At least with its
 complete severity. Installed git on Ubuntu in a virtual machine was
 able to clone the subversion repos past the tag with the white-space
 at the end. I am not sure but apparently this tag has not been
 converted.

 The git repos I could copy from Ubuntu to windows. So far no problems
 seen in this copy on windows.


The cause of the problem is that the tagname isn't a valid filename on
Windows, which git requires it to 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: [RFC] test-lib.sh: No POSIXPERM for cygwin

2013-02-06 Thread Erik Faye-Lund
On Sun, Jan 27, 2013 at 3:57 PM, Torsten Bögershausen tbo...@web.de wrote:
 t0070 and t1301 fail when running the test suite under cygwin.
 Skip the failing tests by unsetting POSIXPERM.


But is this the real reason? I thought Cygwin implemented POSIX permissions...?
--
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/8] Hiding refs

2013-02-06 Thread Michael Haggerty
On 02/05/2013 06:27 PM, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 I would again like to express my discomfort about this feature, which is
 already listed as will merge to next.
 
 Do not take will merge to next too literally.  One major purpose
 of marking a topic as such is exactly to solicit comments like this
 ;-)

I take will merge to next pretty seriously, because I know how hard it
is to get *my* patch series to this state :-)

 * I didn't see a response to Peff's convincing arguments that this
 should be a client-side feature rather than a server-side feature [1].
 
 Uncluttering is not about a choice client should make.  delayed
 advertisement is an orthogonal issue and requires a larger protocol
 update (it needs to make git fetch speak first instead of the
 current protocol in which upload-pack speaks first).

There seem to be a few issues mixed up in this topic.  It is hard to
reason about your patch series without understanding which scenarios and
problems it is meant to address.  First the problems that we might like
to solve:

Clutter: The typical user is subjected to much unneeded clutter in the
 form of references that he/she will likely never use.

Bandwidth: Interactions with the remote repo (clone, fetch, etc) are
   slowed down by the large volume of unnecessary data.

Provenance: Users mistakenly think that content originates with the
repository owner whereas it in fact came from some other
(perhaps untrusted) source.


Now, what are some use-case scenarios in which these problems arise?  As
I understand it, there are a few:

Scenario 1: Some providers junk up their users' repositories with
content that is not created by the repository's owner and that the owner
doesn't want to appear to vouch for (e.g., GitHub pull requests).  These
references might sometimes be useful to fetch, singly or in bulk.

Scenario 2: Some systems junk up their users' repositories with
additional references that are not interesting to most pullers (e.g.,
Gerrit activity markers) though they don't add questionable content.

Scenario 3: Some repository owners might *themselves* want to push
references to their repository but hide them from most users (e.g.,
Junio's topic branches) or make them completely hidden from the rest of
the world (e.g., proprietary vs. open-source branches).

In most of these cases, it would be desirable for at least some users to
be able to fetch and/or push hidden content.

A first weakness of your proposal is that even though the hidden refs
are (optionally) fetchable, there is *no* way to discover them remotely
or to bulk-download them; they would have to be retrieved one by one
using out-of-band information.  And if I understand correctly, there
would be no way to push hidden references remotely (whether manually or
from some automated process).  Such processes would have to be local to
the machine holding the repository.

A second weakness of your proposal is that the repository owner would
*anyway* need local access to the repo server or the help of the
provider to implement reference hiding (since hidden references cannot
be configured remotely).  Who will choose what references to hide?  Most
likely each provider will pick a one-size-fits-all configuration and
apply it to all of the repos that they manage.  All users would be at
the mercy of their provider to make wise choices and would not be able
to override the choice via their client.

A third weakness of your hidden references proposal is that it is
schizophrenic: some references are hidden and undiscoverable, but their
content can nevertheless be made fetchable if the user happens to know
the SHA1.  This is more complicated to understand and reason about than
the rule exactly the content that is referred to by published
references is fetchable.

What would be a better way?  Providers could expose multiple views of
the same repository; for example, one view with just the uncluttered
content, and a second view that includes *all* fetchable references.
Accessing the repository via the first view would give all of the
benefits provided by your hidden reference proposal.  Accessing it via
the second view would allow the hidden references to be fetched (even in
bulk) using purely git tools.  The documentation for the second view
could explain that it contains un-vetted content.

But your proposal does not admit two-tiered access to a single
repository.  You only support one hidden reference configuration that is
applied to all remote access [1].  See below for more ideas about
implementing multiple views.

 * I didn't see a response to my worries that this feature could be
 abused [3].
 
 You can choose not to advertise allow-tip-sha1-in-want capability; I
 do not think it is making things worse than the status quo.

Yes, if the feature is turned off then it is not worse than the status
quo.  But what if the feature is turned on?

Actually, I'm still not 

Re: [PATCH v3 0/8] Hiding refs

2013-02-06 Thread Duy Nguyen
On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 Hiderefs creates a dark corner of a remote git repo that can hold
 arbitrary content that is impossible for anybody to discover but
 nevertheless possible for anybody to download (if they know the name of
 a hidden reference).  In earlier versions of the patch series I believe
 that it was possible to push to a hidden reference hierarchy, which made
 it possible to upload dark content.  The new version appears (from the
 code) to prohibit adding references in a hidden hierarchy, which would
 close the main loophole that I was worried about.  But the documentation
 and the unit tests only explicitly say that updates and deletes are
 prohibited; nothing is said about adding references (unless update is
 understood to include add).  I think the true behavior should be
 clarified and tested.

 I was worried that somehow this dark content could be used for
 malicious purposes; for example, pushing compromised code then
 convincing somebody to download it by SHA1 with the implicit argument
 it's safe since it comes directly from the project's official
 repository.  If it is indeed impossible to populate the dark namespace
 remotely then I can't think of a way to exploit it.

Or you can think hiderefs is the first step to addressing the initial
ref advertisment problem. The series says hidden refs are to be
fetched out of band, but that's not the only way. A new extension can
be added to the protocol later to let the client explore this dark
space. It's only truly dark for old clients. We could even shed some
light to old clients by sending a dummy ref with some loud name like
PLEASE_UPDATE_TO_LATEST_GIT_TO_FETCH_REMAINING_REFS (new clients
silently drop this ref)
--
Duy
--
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] Verify Content-Type from smart HTTP servers

2013-02-06 Thread Michael Schubert
On 01/31/2013 11:09 PM, Junio C Hamano wrote:

  
 -static int http_request_reauth(const char *url, void *result, int target,
 +static int http_request_reauth(const char *url,
 +struct strbuf *type,
 +void *result, int target,
  int options)
  {
 - int ret = http_request(url, result, target, options);
 + int ret = http_request(url, type, result, target, options);
   if (ret != HTTP_REAUTH)
   return ret;
 - return http_request(url, result, target, options);
 + return http_request(url, type, result, target, options);
  }

This needs something like

diff --git a/http.c b/http.c
index d868d8b..da43be3 100644
--- a/http.c
+++ b/http.c
@@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
int ret = http_request(url, type, result, target, options);
if (ret != HTTP_REAUTH)
return ret;
+   if (type)
+   strbuf_reset(type);
return http_request(url, type, result, target, options);
 }

on top. Otherwise we get

text/plainapplication/x-git-receive-pack-advertisement

when doing HTTP auth.

Thanks.

 -int http_get_strbuf(const char *url, struct strbuf *result, int options)
 +int http_get_strbuf(const char *url,
 + struct strbuf *type,
 + struct strbuf *result, int options)
  {
 - return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options);
 + return http_request_reauth(url, type, result,
 +HTTP_REQUEST_STRBUF, options);
  }
  
  /*
 @@ -878,7 +891,7 @@ static int http_get_file(const char *url, const char 
 *filename, int options)
   goto cleanup;
   }
  
 - ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options);
 + ret = http_request_reauth(url, NULL, result, HTTP_REQUEST_FILE, 
 options);
   fclose(result);
  
   if ((ret == HTTP_OK)  move_temp_to_file(tmpfile.buf, filename))
 @@ -904,7 +917,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
   int ret = -1;
  
   url = quote_ref_url(base, ref-name);
 - if (http_get_strbuf(url, buffer, HTTP_NO_CACHE) == HTTP_OK) {
 + if (http_get_strbuf(url, NULL, buffer, HTTP_NO_CACHE) == HTTP_OK) {
   strbuf_rtrim(buffer);
   if (buffer.len == 40)
   ret = get_sha1_hex(buffer.buf, ref-old_sha1);
 @@ -997,7 +1010,7 @@ int http_get_info_packs(const char *base_url, struct 
 packed_git **packs_head)
   strbuf_addstr(buf, objects/info/packs);
   url = strbuf_detach(buf, NULL);
  
 - ret = http_get_strbuf(url, buf, HTTP_NO_CACHE);
 + ret = http_get_strbuf(url, NULL, buf, HTTP_NO_CACHE);
   if (ret != HTTP_OK)
   goto cleanup;
  
 diff --git a/http.h b/http.h
 index 0a80d30..25d1931 100644
 --- a/http.h
 +++ b/http.h
 @@ -132,7 +132,7 @@ extern char *get_remote_object_url(const char *url, const 
 char *hex,
   *
   * If the result pointer is NULL, a HTTP HEAD request is made instead of GET.
   */
 -int http_get_strbuf(const char *url, struct strbuf *result, int options);
 +int http_get_strbuf(const char *url, struct strbuf *content_type, struct 
 strbuf *result, int options);
  
  /*
   * Prints an error message using error() containing url and curl_errorstr,
 diff --git a/remote-curl.c b/remote-curl.c
 index 9a8b123..e6f3b63 100644
 --- a/remote-curl.c
 +++ b/remote-curl.c
 @@ -92,6 +92,8 @@ static void free_discovery(struct discovery *d)
  
  static struct discovery* discover_refs(const char *service)
  {
 + struct strbuf exp = STRBUF_INIT;
 + struct strbuf type = STRBUF_INIT;
   struct strbuf buffer = STRBUF_INIT;
   struct discovery *last = last_discovery;
   char *refs_url;
 @@ -113,7 +115,7 @@ static struct discovery* discover_refs(const char 
 *service)
   }
   refs_url = strbuf_detach(buffer, NULL);
  
 - http_ret = http_get_strbuf(refs_url, buffer, HTTP_NO_CACHE);
 + http_ret = http_get_strbuf(refs_url, type, buffer, HTTP_NO_CACHE);
   switch (http_ret) {
   case HTTP_OK:
   break;
 @@ -133,16 +135,19 @@ static struct discovery* discover_refs(const char 
 *service)
   last-buf = last-buf_alloc;
  
   if (maybe_smart  5 = last-len  last-buf[4] == '#') {
 - /* smart HTTP response; validate that the service
 + /*
 +  * smart HTTP response; validate that the service
* pkt-line matches our request.
*/
 - struct strbuf exp = STRBUF_INIT;
 -
 + strbuf_addf(exp, application/x-%s-advertisement, service);
 + if (strbuf_cmp(exp, type))
 + die(invalid content-type %s, type.buf);
   if (packet_get_line(buffer, last-buf, last-len) = 0)
   die(%s has invalid packet header, refs_url);
   if (buffer.len  buffer.buf[buffer.len - 1] == '\n')
   

Re: [PATCH] Verify Content-Type from smart HTTP servers

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 11:24:41AM +0100, Michael Schubert wrote:

 On 01/31/2013 11:09 PM, Junio C Hamano wrote:
 
   
  -static int http_request_reauth(const char *url, void *result, int target,
  +static int http_request_reauth(const char *url,
  +  struct strbuf *type,
  +  void *result, int target,
 int options)
   {
  -   int ret = http_request(url, result, target, options);
  +   int ret = http_request(url, type, result, target, options);
  if (ret != HTTP_REAUTH)
  return ret;
  -   return http_request(url, result, target, options);
  +   return http_request(url, type, result, target, options);
   }
 
 This needs something like
 
 diff --git a/http.c b/http.c
 index d868d8b..da43be3 100644
 --- a/http.c
 +++ b/http.c
 @@ -860,6 +860,8 @@ static int http_request_reauth(const char *url,
 int ret = http_request(url, type, result, target, options);
 if (ret != HTTP_REAUTH)
 return ret;
 +   if (type)
 +   strbuf_reset(type);
 return http_request(url, type, result, target, options);
  }
 
 on top. Otherwise we get
 
 text/plainapplication/x-git-receive-pack-advertisement
 
 when doing HTTP auth.

Good catch. It probably makes sense to put it in http_request, so that
we also protect against any existing cruft from the callers of
http_get_*, like:

-- 8 --
Subject: [PATCH] http_request: reset type strbuf before adding

Callers may pass us a strbuf which we use to record the
content-type of the response. However, we simply appended to
it rather than overwriting its contents, meaning that cruft
in the strbuf gave us a bogus type. E.g., the multiple
requests triggered by http_request could yield a type like
text/plainapplication/x-git-receive-pack-advertisement.

Reported-by: Michael Schubert msc...@elegosoft.com
Signed-off-by: Jeff King p...@peff.net
---
Is it worth having a strbuf_set* family of functions to match the
strbuf_add*? We seem to have these sorts of errors with strbuf from time
to time, and I wonder if that would make it easier (and more readable)
to do the right thing.

 http.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/http.c b/http.c
index d868d8b..d9d1aad 100644
--- a/http.c
+++ b/http.c
@@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf 
*type,
 
if (type) {
char *t;
+   strbuf_reset(type);
curl_easy_getinfo(slot-curl, CURLINFO_CONTENT_TYPE, t);
if (t)
strbuf_addstr(type, t);
-- 
1.8.1.2.11.g1a2f572

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation

2013-02-06 Thread Duy Nguyen
On Wed, Feb 6, 2013 at 11:35 AM, Junio C Hamano gits...@pobox.com wrote:
 How about this since [PATCH v3]:

 diff --git a/utf8.c b/utf8.c
 index 52dbd..b893a 100644
 --- a/utf8.c
 +++ b/utf8.c
 @@ -443,8 +443,11 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
 strbuf_vaddf(buf, format, arg);
 va_end (arg);

 -   fputs(buf.buf, stream);
 -   columns = utf8_strwidth(buf.buf);
 +   columns = fputs(buf.buf, stream);
 +   /* If no error occurs, and really write something (columns  0),
 +* calculate really columns width with utf8_strwidth. */
 +   if (columns  0)
 +   columns = utf8_strwidth(buf.buf);
 strbuf_release(buf);
 return columns;
  }

 The above bugfix does not address my original concern about
 the name, though.

How about utf8_fwprintf? wprintf() deals with wide characters and
returns the number of wide characters, I think the name fits. And we
could just drop utf8_ and use the existing name, because we don't use
wchar_t* anyway.
-- 
Duy
--
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 3/8] upload/receive-pack: allow hiding ref hierarchies

2013-02-06 Thread Jeff King
On Tue, Feb 05, 2013 at 07:45:01AM -0800, Junio C Hamano wrote:

  In the earlier review, I mentioned making this per-service, but I see
  that is not the case here. Do you have an argument against doing so?
 
 Perhaps then I misunderstood your intention.  By reminding me of the
 receive-pack side, I thought you were hinting to unify these two
 into one, which I did.  There is no argument against it.

What I meant was that there should be transfer.hiderefs, and an
individual {receive,uploadpack}.hiderefs, similar to the way we have
transfer.unpacklimit. That makes the easy case (hiding the refs
completely) easy, but leaves the flexibility for more.

Like this:

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a8248d9..131c163 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -59,7 +59,7 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
 
 static int receive_pack_config(const char *var, const char *value, void *cb)
 {
-   int status = parse_hide_refs_config(var, value, cb);
+   int status = parse_hide_refs_config(var, value, receive);
 
if (status)
return status;
diff --git a/refs.c b/refs.c
index e3574ca..9bfea58 100644
--- a/refs.c
+++ b/refs.c
@@ -2560,9 +2560,13 @@ int parse_hide_refs_config(const char *var, const char 
*value, void *unused)
 
 static struct string_list *hide_refs;
 
-int parse_hide_refs_config(const char *var, const char *value, void *unused)
+int parse_hide_refs_config(const char *var, const char *value, void *vsection)
 {
-   if (!strcmp(transfer.hiderefs, var)) {
+   const char *section = vsection;
+
+   if (!strcmp(transfer.hiderefs, var) ||
+   (!prefixcmp(var, section) 
+!strcmp(var + strlen(section), .hiderefs))) {
char *ref;
int len;
 
diff --git a/upload-pack.c b/upload-pack.c
index 37977e2..c0390af 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -794,7 +794,7 @@ static int upload_pack_config(const char *var, const char 
*value, void *unused)
 {
if (!strcmp(uploadpack.allowtipsha1inwant, var))
allow_tip_sha1_in_want = git_config_bool(var, value);
-   return parse_hide_refs_config(var, value, unused);
+   return parse_hide_refs_config(var, value, uploadpack);
 }
 
 int main(int argc, char **argv)


As an aside, I wonder if there is any point to the void pointer
parameter of parse_hide_refs_config. It is not used as a git_config
callback anywhere.

  And I
  have not seen complaints about the current system.
 
 Immediately after I added github to the set of places I push into,
 which I think is long before you joined GitHub, I noticed that _my_
 repository gets contaminated by second rate commits called pull
 requests, and I may even have complained, but most likely I didn't,
 as I could easily tell that, even though I know it is _not_ the only
 way, nor even the best way [*1*], to implement the GitHub's pull
 request workflow, I perfectly well understood that it would be the
 most expedient way for GitHub folks to implement this feature.
 
 I think you should take lack of complaints with a huge grain of
 salt.  It does not suggest much.

Sure, I do not pretend that nobody cares. But it is certainly not a
pressing issue, or there would probably be more outcry. And we must also
weigh it against the silent majority that are perfectly happy with the
status quo, that lets them fetch refs/pull/* as any other ref.

In your case, I really think the problem is less that you have a problem
with PR refs in the repository, and more that you do not care about the
pull request feature at all. To you it is useless noise, both in the
repo and on the web. Your arguments about provenance could apply equally
well to PRs accessible via the web interface.

I think the refs/ clutter is only an issue if you want to do mirroring,
and then you have an obvious conflict: did the fetcher want to mirror
everything, including refs/pull, or do they consider that to be clutter?
Only the client knows, which is why I think refspecs are the right place
to deal with clutter (the fact that we cannot say everything except
refs/pull/* is a weakness in our refspecs).

 *1* From the ownership point of view, objects that are only
 reachable from these refs/pull/* refs do *not* belong to the
 requestee, until the requestee chooses to accept the changes.
 
 A malicious requestor can fork your repository, add an objectionable
 blob to it, and throw a pull request at you.  GitHub shows that the
 blob now belongs to your repository, so the requestor turns around
 and file a DMCA takedown complaint against your repository.  A
 clueful judge would then agree with the complaint after running a
 clone --mirror and seeing the blob in your repository.  Oops?

I don't think this is a problem in practice. DMCA notices do not go to
the repository owner; they go to GitHub. And as far as I know, our
support staff deals with them on a case 

Why is ident_is_sufficient different on Windows?

2013-02-06 Thread Max Horn
Hi there,

while trying to understand which parts of the author  committer identity are 
mandatory (name, email, or both), I ended up in ident.c, looking at 
ident_is_sufficient(), and to my surprise discovered that this seems to differ 
between Windows (were both are mandatory) and everyone else:

static int ident_is_sufficient(int user_ident_explicitly_given)
{
#ifndef WINDOWS
return (user_ident_explicitly_given  IDENT_MAIL_GIVEN);
#else
return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
#endif
}


According to git blame, this was introduced here:

commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e
Author: Junio C Hamano gits...@pobox.com
Date:   Sun Jan 17 13:54:28 2010 -0800

user_ident_sufficiently_given(): refactor the logic to be usable from 
elsewhere


The commit message sounds as if this was only a refactoring, but the patch to 
me look as if it changes behaviour, too. Of course this could very well be 
false, say due to code elsewhere that already caused Windows to behave 
differently; I wouldn't know.


Still, I wonder: Why does this difference exist?


Cheers,
Max


--
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-send-email: add ~/.authinfo parsing

2013-02-06 Thread Michal Nazarewicz
On Wed, Feb 06 2013, Junio C Hamano gits...@pobox.com wrote:
 I see a lot of rerolls on the credential helper front, but is there
 anybody working on hooking send-email to the credential framework?

I assumed someone had, but if not I can take a stab at it.  I'm not sure
however how should I map server, server-port, and user to credential
key-value pairs.  I'm leaning towards

protocol=smtp
host=smtp-server:smtp-port
user=user

and than netrc/authinfo helper splitting host to host name and port
number, unless port is not in host in which case protocol is assumed as
port.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpyBEG11AUhI.pgp
Description: PGP signature


Re: [PATCH] git-send-email: add ~/.authinfo parsing

2013-02-06 Thread Ted Zlatanov
On Wed, 06 Feb 2013 14:26:46 +0100 Michal Nazarewicz min...@mina86.com wrote: 

MN On Wed, Feb 06 2013, Junio C Hamano gits...@pobox.com wrote:
 I see a lot of rerolls on the credential helper front, but is there
 anybody working on hooking send-email to the credential framework?

MN I assumed someone had, but if not I can take a stab at it.  I'm not sure
MN however how should I map server, server-port, and user to credential
MN key-value pairs.  I'm leaning towards

MN protocol=smtp
MN host=smtp-server:smtp-port
MN user=user

MN and than netrc/authinfo helper splitting host to host name and port
MN number, unless port is not in host in which case protocol is assumed as
MN port.

That would work (with my PATCHv6 of the netrc credential helper) as
follows:

1) just host

host=H

maps to

machine H login Y password Z

2) host + protocol smtp

host=H
protocol=smtp

maps to any of:

machine H port smtp login Y password Z
machine H protocol smtp login Y password Z

3) host:port + protocol smtp

host=H:25
protocol=smtp

maps to any of:

machine H port 25 protocol smtp login Y password Z
machine H:25 port smtp login Y password Z
machine H:25 protocol smtp login Y password Z

That's my understanding of what we discussed with Peff and Junio about
token mapping.  Note we don't split the input host, but instead say if
token 'port' is numeric, append it to the host token on the netrc side.

Does that sound reasonable?  If yes, I can add it to the testing
Makefile for the netrc credential helper, to make sure it's clearly
stated and tested.

Ted
--
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-send-email: add ~/.authinfo parsing

2013-02-06 Thread Ted Zlatanov
On Wed, 06 Feb 2013 09:11:17 +0100 Matthieu Moy matthieu@grenoble-inp.fr 
wrote: 

MM Junio C Hamano gits...@pobox.com writes:
 I see a lot of rerolls on the credential helper front, but is there
 anybody working on hooking send-email to the credential framework?

MM Not answering the question, but git-remote-mediawiki supports the
MM credential framework. It is written in perl, and the credential support
MM is rather cleanly written and doesn't have dependencies on the wiki
MM part, so the way to go for send-email is probably to libify the
MM credential support in git-remote-mediawiki, and to use it in send-email.

I looked and that's indeed very useful.  If it's put in a library, I'd
use credential_read() and credential_write() in my netrc credential
helper.  But I would formalize it a little more about the token names
and output, and I wouldn't necessarily die() on error.  Maybe this can
be merged with the netrc credential helper's
read_credential_data_from_stdin() and print_credential_data()?

Let me know if you'd like me to libify this...  I'm happy to leave it to
Matthieu or Michal, or anyone else interested.

Ted
--
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 in git log --graph -p -m (version 1.7.7.6)

2013-02-06 Thread Dale R. Worley
 From: Matthieu Moy matthieu@grenoble-inp.fr
 
 In any case, I can't reproduce with 1.8.1.2.526.gf51a757: I don't get
 undless output. On the other hand, I get a slightly misformatted output:
 
 *   commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 
 2c1e6a36f4b712e914fac994463da7d0fdb2bc6d)
 |\  Merge: 2c1e6a3 33e70e7
 | | Author: Matthieu Moy matthieu@imag.fr
 | | Date:   Tue Feb 5 22:05:33 2013 +0100
 | | 
 | | Commit S
 | | 
 | | diff --git a/file b/file
 | | index 6bb4d3e..afd2e75 100644
 | | --- a/file
 | | +++ b/file
 | | @@ -1,4 +1,5 @@
 | |  1
 | |  1a
 | |  2
 | | +2a
 | |  3
 | | 
 commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 
 33e70e70c0173d634826b998bdc304f93c0966b8)
 | | Merge: 2c1e6a3 33e70e7
 | | Author: Matthieu Moy matthieu@imag.fr
 | | Date:   Tue Feb 5 22:05:33 2013 +0100
 
 The second commit line (diff with second parent) doesn't have the
 | | prefix, I don't think this is intentional.

The second commit line should start with | * :

 | | 
 | * commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 
 33e70e70c0173d634826b998bdc304f93c0966b8)
 | | Merge: 2c1e6a3 33e70e7
 | | Author: Matthieu Moy matthieu@imag.fr
 | | Date:   Tue Feb 5 22:05:33 2013 +0100

Dale
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH 0/4] textconv for show and grep

2013-02-06 Thread Michael J Gruber
This min series aims at completing the textconv support of user facing
commands. It is RFC for lack of documentation and tests, to check
whether we really want to go in that direction (I do).

1/4 covers the missing textconv support in the blob case of git
show, which should be (and then is) analogous to cat-file and diff.

2/4 remedies an oddity of git cat-file --textconv, which errored out
when there is no textconv filter rather than giving an unfiltered blob
(like every other textconv aware command).

3/4 implements --textconv for git grep sans the blob case; the code
is all Jeff's.

4/4 adds blob support to 3/4 (the rev:path case).

3 and 4 can be squashed, of course.

Now I'm quite a happy differ/shower/grepper with my latin1 and OO files ;)

Jeff King (1):
  grep: allow to use textconv filters

Michael J Gruber (3):
  show: obey --textconv for blobs
  cat-file: do not die on --textconv without textconv filters
  grep: obey --textconv for the case rev:path

 builtin/cat-file.c   |   9 ++--
 builtin/grep.c   |  13 +++---
 builtin/log.c|  24 +--
 grep.c   | 100 +--
 grep.h   |   1 +
 object.c |  26 ---
 object.h |   2 +
 t/t8007-cat-file-textconv.sh |  20 +++--
 8 files changed, 148 insertions(+), 47 deletions(-)

-- 
1.8.1.2.752.g32d147e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH 1/4] show: obey --textconv for blobs

2013-02-06 Thread Michael J Gruber
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.

Signed-off-by: Michael J Gruber g...@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;
-- 
1.8.1.2.752.g32d147e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters

2013-02-06 Thread Michael J Gruber
When a command is supposed to use textconv filters (by default or with
--textconv) and none are configured then the blob is output without
conversion; the only exception to this rule is cat-file --textconv.

Make it behave like the rest of textconv aware commands.

Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
 builtin/cat-file.c   |  9 +
 t/t8007-cat-file-textconv.sh | 20 +---
 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 00528dd..6912dc2 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -146,10 +146,11 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name)
die(git cat-file --textconv %s: object must be 
sha1:path,
obj_name);
 
-   if (!textconv_object(obj_context.path, obj_context.mode, sha1, 
1, buf, size))
-   die(git cat-file --textconv: unable to run textconv on 
%s,
-   obj_name);
-   break;
+   if (textconv_object(obj_context.path, obj_context.mode, sha1, 
1, buf, size))
+   break;
+
+   /* otherwise expect a blob */
+   exp_type = blob;
 
case 0:
if (type_from_string(exp_type) == OBJ_BLOB) {
diff --git a/t/t8007-cat-file-textconv.sh b/t/t8007-cat-file-textconv.sh
index 78a0085..83c6636 100755
--- a/t/t8007-cat-file-textconv.sh
+++ b/t/t8007-cat-file-textconv.sh
@@ -22,11 +22,11 @@ test_expect_success 'setup ' '
 '
 
 cat expected EOF
-fatal: git cat-file --textconv: unable to run textconv on :one.bin
+bin: test version 2
 EOF
 
 test_expect_success 'no filter specified' '
-   git cat-file --textconv :one.bin 2result
+   git cat-file --textconv :one.bin result 
test_cmp expected result
 '
 
@@ -36,10 +36,6 @@ test_expect_success 'setup textconv filters' '
git config diff.test.cachetextconv false
 '
 
-cat expected EOF
-bin: test version 2
-EOF
-
 test_expect_success 'cat-file without --textconv' '
git cat-file blob :one.bin result 
test_cmp expected result
@@ -73,25 +69,19 @@ test_expect_success 'cat-file --textconv on previous 
commit' '
 '
 
 test_expect_success SYMLINKS 'cat-file without --textconv (symlink)' '
+   printf %s one.bin expected 
git cat-file blob :symlink.bin result 
-   printf %s one.bin expected
test_cmp expected result
 '
 
 
 test_expect_success SYMLINKS 'cat-file --textconv on index (symlink)' '
-   ! git cat-file --textconv :symlink.bin 2result 
-   cat expected \EOF 
-fatal: git cat-file --textconv: unable to run textconv on :symlink.bin
-EOF
+   git cat-file --textconv :symlink.bin result 
test_cmp expected result
 '
 
 test_expect_success SYMLINKS 'cat-file --textconv on HEAD (symlink)' '
-   ! git cat-file --textconv HEAD:symlink.bin 2result 
-   cat expected EOF 
-fatal: git cat-file --textconv: unable to run textconv on HEAD:symlink.bin
-EOF
+   git cat-file --textconv HEAD:symlink.bin result 
test_cmp expected result
 '
 
-- 
1.8.1.2.752.g32d147e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH 4/4] grep: obey --textconv for the case rev:path

2013-02-06 Thread Michael J Gruber
Make grep obey the --textconv option also for the object case, i.e.
when used with an argument rev:path.

Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
 builtin/grep.c | 11 ++-
 object.c   | 26 --
 object.h   |  2 ++
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 915c8ef..0f3c4db 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -458,10 +458,10 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
 }
 
 static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
-  struct object *obj, const char *name)
+  struct object *obj, const char *name, struct 
object_context *oc)
 {
if (obj-type == OBJ_BLOB)
-   return grep_sha1(opt, obj-sha1, name, 0, NULL);
+   return grep_sha1(opt, obj-sha1, name, 0, oc ? oc-path : NULL);
if (obj-type == OBJ_COMMIT || obj-type == OBJ_TREE) {
struct tree_desc tree;
void *data;
@@ -503,7 +503,7 @@ static int grep_objects(struct grep_opt *opt, const struct 
pathspec *pathspec,
for (i = 0; i  nr; i++) {
struct object *real_obj;
real_obj = deref_tag(list-objects[i].item, NULL, 0);
-   if (grep_object(opt, pathspec, real_obj, 
list-objects[i].name)) {
+   if (grep_object(opt, pathspec, real_obj, list-objects[i].name, 
list-objects[i].context)) {
hit = 1;
if (opt-status_only)
break;
@@ -820,14 +820,15 @@ int cmd_grep(int argc, const char **argv, const char 
*prefix)
for (i = 0; i  argc; i++) {
const char *arg = argv[i];
unsigned char sha1[20];
+   struct object_context oc;
/* Is it a rev? */
-   if (!get_sha1(arg, sha1)) {
+   if (!get_sha1_with_context(arg, 0, sha1, oc)) {
struct object *object = parse_object(sha1);
if (!object)
die(_(bad object %s), arg);
if (!seen_dashdash)
verify_non_filename(prefix, arg);
-   add_object_array(object, arg, list);
+   add_object_array_with_context(object, arg, list, 
xmemdupz(oc, sizeof(struct object_context)));
continue;
}
if (!strcmp(arg, --)) {
diff --git a/object.c b/object.c
index 4af3451..1b796c7 100644
--- a/object.c
+++ b/object.c
@@ -245,12 +245,7 @@ int object_list_contains(struct object_list *list, struct 
object *obj)
return 0;
 }
 
-void add_object_array(struct object *obj, const char *name, struct 
object_array *array)
-{
-   add_object_array_with_mode(obj, name, array, S_IFINVALID);
-}
-
-void add_object_array_with_mode(struct object *obj, const char *name, struct 
object_array *array, unsigned mode)
+static void add_object_array_with_mode_context(struct object *obj, const char 
*name, struct object_array *array, unsigned mode, struct object_context 
*context)
 {
unsigned nr = array-nr;
unsigned alloc = array-alloc;
@@ -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;
 }
 
+void add_object_array(struct object *obj, const char *name, struct 
object_array *array)
+{
+   add_object_array_with_mode(obj, name, array, S_IFINVALID);
+}
+
+void add_object_array_with_mode(struct object *obj, const char *name, struct 
object_array *array, unsigned mode)
+{
+   add_object_array_with_mode_context(obj, name, array, mode, NULL);
+}
+
+void add_object_array_with_context(struct object *obj, const char *name, 
struct object_array *array, struct object_context *context)
+{
+   if (context)
+   add_object_array_with_mode_context(obj, name, array, 
context-mode, context);
+   else
+   add_object_array_with_mode_context(obj, name, array, 
S_IFINVALID, context);
+}
+
 void object_array_remove_duplicates(struct object_array *array)
 {
unsigned int ref, src, dst;
diff --git a/object.h b/object.h
index 6a97b6b..a11d719 100644
--- a/object.h
+++ b/object.h
@@ -13,6 +13,7 @@ struct object_array {
struct object *item;
const char *name;
unsigned mode;
+   struct object_context *context;
} *objects;
 };
 
@@ -74,6 +75,7 @@ int object_list_contains(struct object_list *list, struct 
object *obj);
 /* Object array handling .. */
 void add_object_array(struct object *obj, const char *name, struct 
object_array *array);
 void add_object_array_with_mode(struct object *obj, const char *name, 

CodingGuidelines Perl amendment (was: [PATCH 1/3] Add contrib/credentials/netrc with GPG support)

2013-02-06 Thread Ted Zlatanov
On Mon, 04 Feb 2013 15:10:45 -0800 Junio C Hamano gits...@pobox.com wrote: 

JCH Ted Zlatanov t...@lifelogs.com writes:
JCH I thought that we tend to avoid Emacs/Vim formatting cruft left in
JCH the file.  Do we have any in existing file outside contrib/?
 
 No, but it's a nice way to express the settings so no one is guessing
 what the project prefers.  At least for me it's not an issue anymore,
 since I understand your criteria better now, so let me know if you want
 me to express it in the CodingGuidelines, in a dir-locals.el file, or
 somewhere else.

JCH Historically we treated this from CodingGuidelines a sufficient
JCH clue:

JCH As for more concrete guidelines, just imitate the existing code
JCH (this is a good guideline, no matter which project you are
JCH contributing to). It is always preferable to match the _local_
JCH convention. New code added to git suite is expected to match
JCH the overall style of existing code. Modifications to existing
JCH code is expected to match the style the surrounding code already
JCH uses (even if it doesn't match the overall style of existing code).

JCH but over time people wanted more specific guidelines and added
JCH language specific style guides there.  We have sections that cover
JCH C, shell and Python, and I do not think adding Perl would not hurt.

The following is how I have interpreted the Perl guidelines.  I hope
it's OK to include Emacs-specific settings; they make it much easier to
reindent code to be acceptable.

I will submit as a patch if you think this is reasonable at all.

The org-mode markers around the code are just a suggestion.

For Perl 5 programs:

 - Most of the C guidelines above apply.

 - We try to support Perl 5.8 and later (use Perl 5.008).

 - use strict and use warnings are strongly preferred.

 - As in C (see above), we avoid using braces unnecessarily (but Perl
   forces braces around if/unless/else/foreach blocks, so this is not
   always possible).

 - Don't abuse statement modifiers (unless $youmust).

 - We try to avoid assignments inside if().

 - Learn and use Git.pm if you need that functionality.

 - For Emacs, it's useful to put the following in
   GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:

#+begin_src lisp
((nil . ((indent-tabs-mode . t)
  (tab-width . 8)
  (fill-column . 80)))
 (cperl-mode . ((cperl-indent-level . 8)
(cperl-extra-newline-before-brace . nil)
(cperl-merge-trailing-else . t
#+end_src

--
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] Verify Content-Type from smart HTTP servers

2013-02-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Is it worth having a strbuf_set* family of functions to match the
 strbuf_add*? We seem to have these sorts of errors with strbuf from time
 to time, and I wonder if that would make it easier (and more readable)
 to do the right thing.

Possibly.

The callsite below may be a poor example, though; you would need the
_reset() even if you change the _addstr() we can see in the context
to _setstr() to make sure later strbuf_*(type) will start from a
clean slate when !t anyway, no?


  http.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/http.c b/http.c
 index d868d8b..d9d1aad 100644
 --- a/http.c
 +++ b/http.c
 @@ -841,6 +841,7 @@ static int http_request(const char *url, struct strbuf 
 *type,
  
   if (type) {
   char *t;
 + strbuf_reset(type);
   curl_easy_getinfo(slot-curl, CURLINFO_CONTENT_TYPE, t);
   if (t)
   strbuf_addstr(type, t);
--
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 3/8] upload/receive-pack: allow hiding ref hierarchies

2013-02-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Feb 05, 2013 at 07:45:01AM -0800, Junio C Hamano wrote:

  In the earlier review, I mentioned making this per-service, but I see
  that is not the case here. Do you have an argument against doing so?
 
 Perhaps then I misunderstood your intention.  By reminding me of the
 receive-pack side, I thought you were hinting to unify these two
 into one, which I did.  There is no argument against it.

 What I meant was that there should be transfer.hiderefs, and an
 individual {receive,uploadpack}.hiderefs, similar to the way we have
 transfer.unpacklimit.

Yes, as I said, I misunderstood your intention.
--
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


How to diff 2 file revisions with gitk

2013-02-06 Thread R. Diez
Hi there:

I asked a few days ago whether I could easily diff 2 file revisions with the 
mouse in gitk, but I got no reply yet, see here:


   How to diff two file revisions with the mouse (with gitk)
   https://groups.google.com/forum/#!topic/git-users/9znsQsTB0dE

I am hoping that it was the wrong mailing list, and this one the right one. 8-)

Here is the full question text again:

8888

I would like to start gitk, select with the mouse 2 
revisions of some file and then compare them, hopefully with an external
 diff tool, very much like I am used to with WinCVS.

The closest I
 got is to start gitk with a filename as an argument, in order to 
restrict the log to that one file. Then I right-click on a commit (a 
file revision) and choose Mark this commit. However, if I right-click 
on another commit and choose Compare with marked commit, I get a full 
commit diff with all files, and not just the file I specified on the 
command-line arguments.

Selecting a filename in the Tree view and choosing Highlight this only, as 
I found on the Internet, does not seem to help.

I have git 1.7.9 (on Cygwin). Can someone help?

By the way, it would be nice if gitk could launch the external diff tool from 
the Compare with marked commit option too.

8888

Thanks in advance,
   rdiez
--
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-send-email: add ~/.authinfo parsing

2013-02-06 Thread Ted Zlatanov
On Wed, 06 Feb 2013 16:10:12 +0100 Matthieu Moy matthieu@grenoble-inp.fr 
wrote: 

MM Ted Zlatanov t...@lifelogs.com writes:
MM [...] so the way to go for send-email is probably to libify the
MM credential support in git-remote-mediawiki, and to use it in send-email.
 
 I looked and that's indeed very useful.  If it's put in a library, I'd
 use credential_read() and credential_write() in my netrc credential
 helper.  But I would formalize it a little more about the token names
 and output,

MM Can you elaborate on this? The idea of the Perl code was to mimick a
MM call to the C API, keeping essentially the same names.

None of these are a big deal, and Michal said he's working on libifying
this anyhow:

- making 'fill' a special operation is weird
- anchor the key regex to beginning of line (not strictly necessary)
- sort the output tokens (after 'url' is extracted) so the output is consistent 
and testable

 and I wouldn't necessarily die() on error. 

MM Sure, die()ing in a library is bad.

 Maybe this can be merged with the netrc credential helper's
 read_credential_data_from_stdin() and print_credential_data()?

MM I don't know about the netrc credential helper, but I guess that's
MM another layer. The git-remote-mediawiki code is the code to call the
MM credential C API, that in turn may (or may not) call a credential
MM helper.

Yup.  But what you call read and write are, to the credential
helper, write and read but it's the same protocol :)  So maybe the
names should be changed to reflect that, e.g. query and response.

MM One thing to be careful about: git-remote-mediawiki is currently a
MM standalone script, so it can be installed with a plain cp
MM git-remote-mediawiki $somewhere/.  One consequence of libification
MM is that it adds a dependency on the library (e.g. Git.pm). We should
MM be carefull to keep it easy for the user to install it (e.g. some
MM kind of make install, or update the doc).

I don't know--it's up to the `git-remote-mediawiki' maintainers...  But
I think anywhere you have Git, you also have Git.pm, right?  Maybe?  But
then you also have to look at whether Git.pm has the functionality you
need... so I better go quiet :)

Ted
--
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


[Request] Git export with hardlinks

2013-02-06 Thread Thomas Koch
Hi,

I'd like to script a git export command that can be given a list of already 
exported worktrees and the tree SHA1s these worktrees correspond too. The git 
export command should then for every file it wants to export lookup in the 
existing worktrees whether an identical file is already present and in that 
case hardlink to the new export location instead of writing the same file 
again.

Use Case: A git based web deployment system that exports git trees to be 
served by a web server. Every new deployment is written to a new folder. After 
the export the web server should start serving new requests from the new 
folder.

It might be possible that this is premature optimization. But I'd like to 
learn more Python and dulwich by hacking this.

Do you have any additional thoughts or use cases about this?

Regards,

Thomas Koch, http://www.koch.ro
--
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: CodingGuidelines Perl amendment

2013-02-06 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

  - As in C (see above), we avoid using braces unnecessarily (but Perl
forces braces around if/unless/else/foreach blocks, so this is not
always possible).

Is it ever (as opposed to not always) possible to omit braces?

It sounds as if we encourage the use of statement modifiers, which
certainly is not what I want to see.

You probably would want to mention that opening braces for
if/else/elsif do not sit on their own line, and closing braces for
them will be followed the next else/elseif on the same line
instead, but that is part of most of the C guidelines above apply
so it may be redundant.

  - Don't abuse statement modifiers (unless $youmust).

It does not make a useful guidance to leave $youmust part
unspecified.

Incidentally, your sentence is a good example of where use of
statement modifiers is appropriate: $youmust is rarely true.

In general:

... do something ...
do_this() unless (condition);
... do something else ...

is easier to follow the flow of the logic than

... do something ...
unless (condition) {
do_this();
}
... do something else ...

*only* when condition is extremely rare, iow, when do_this() is
expected to be almost always called.
--
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-send-email: add ~/.authinfo parsing

2013-02-06 Thread Matthieu Moy
Ted Zlatanov t...@lifelogs.com writes:

 None of these are a big deal, and Michal said he's working on libifying
 this anyhow:

 - making 'fill' a special operation is weird

Well, 'fill' is the only operation that mutates the credential structure
(i.e. the only one for which git credential emits an output to be
parsed), so you don't have much choice.

 - anchor the key regex to beginning of line (not strictly necessary)

Right. The greedyness of * ensures correction, but I like explicit
anchors ^...$ too.

 - sort the output tokens (after 'url' is extracted) so the output is 
 consistent and testable

Why not, if you want to use the output of credential_write in tests. But
credential_write is essentially used to talk to git credential, so the
important information is the content of the hash before credential_write
and after credential_read. They are unordered, but consistent and
testable.

 Maybe this can be merged with the netrc credential helper's
 read_credential_data_from_stdin() and print_credential_data()?

 MM I don't know about the netrc credential helper, but I guess that's
 MM another layer. The git-remote-mediawiki code is the code to call the
 MM credential C API, that in turn may (or may not) call a credential
 MM helper.

 Yup.  But what you call read and write are, to the credential
 helper, write and read but it's the same protocol :)  So maybe the
 names should be changed to reflect that, e.g. query and response.

I don't think that would be a better naming. Maybe serialize and
parse would be better, but query would sound like it establishes the
connection and possibly reads the response to me.

 MM One thing to be careful about: git-remote-mediawiki is currently a
 MM standalone script, so it can be installed with a plain cp
 MM git-remote-mediawiki $somewhere/.  One consequence of libification
 MM is that it adds a dependency on the library (e.g. Git.pm). We should
 MM be carefull to keep it easy for the user to install it (e.g. some
 MM kind of make install, or update the doc).

 I don't know--it's up to the `git-remote-mediawiki' maintainers...

That is, me ;-).

 But I think anywhere you have Git, you also have Git.pm, right?

Yes, but you have to find out where it is installed. Git's Makefile
hardcodes the path to Git.pm at build time, inserting one line in the
perl script:

use lib (split(/:/, $ENV{GITPERLLIB} || $INSTLIBDIR));

The same needs to be done for git-remote-mediawiki. As much as possible,
I'd rather avoid copy-pasting from Git's Makefile, so this means
extracting the perl part of Git's Makefile and make it available in
contrib/.

I'll try a patch in this direction.

 Maybe? But then you also have to look at whether Git.pm has the
 functionality you need...

If git-remote-mediawiki is installed from Git's source, I think it's OK
to assume that Git.pm will be up to date, but that would be even better
if we can issue a clean error message when the functions to be called do
not exist.

-- 
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: [RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters

2013-02-06 Thread Junio C Hamano
Michael J Gruber g...@drmicha.warpmail.net writes:

 When a command is supposed to use textconv filters (by default or with
 --textconv) and none are configured then the blob is output without
 conversion; the only exception to this rule is cat-file --textconv.

I am of two minds.  Because cat-file is mostly a low-level plumbing,
I do not necessarily think it is a bad behaviour for it to error out
when it was asked to apply textconv where there is no filter or when
the filter fails to produce an output.  On the other hand, it
certainly makes it more convenient for callers that do not care too
deeply, taking textconv as a mere hint just like Porcelains do.


 Make it behave like the rest of textconv aware commands.

 Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
 ---
  builtin/cat-file.c   |  9 +
  t/t8007-cat-file-textconv.sh | 20 +---
  2 files changed, 10 insertions(+), 19 deletions(-)

 diff --git a/builtin/cat-file.c b/builtin/cat-file.c
 index 00528dd..6912dc2 100644
 --- a/builtin/cat-file.c
 +++ b/builtin/cat-file.c
 @@ -146,10 +146,11 @@ static int cat_one_file(int opt, const char *exp_type, 
 const char *obj_name)
   die(git cat-file --textconv %s: object must be 
 sha1:path,
   obj_name);
  
 - if (!textconv_object(obj_context.path, obj_context.mode, sha1, 
 1, buf, size))
 - die(git cat-file --textconv: unable to run textconv on 
 %s,
 - obj_name);
 - break;
 + if (textconv_object(obj_context.path, obj_context.mode, sha1, 
 1, buf, size))
 + break;
 +
 + /* otherwise expect a blob */
 + exp_type = blob;

Please use the constant string blob_type that is available for all
callers including this one.

But stepping back a bit.

What happens when I say cat-file -c HEAD:Documentation, and what
should happen when I do so?

I think what we want to see in the ideal world might be:

 * If we have a textconv for tree objects at that path to format it
   specially, textconv_object() may be allowed to textualize it
   (even though it is not a blob, and textconv so far has always
   been about blobs; it needs to be considered carefully if it makes
   sense to allow such a usage) and show it;

 * If we don't, we act as if -c were -p; in other words, we treat
   the built-in human output implemented by cat-file -p as if
   that is a textconv.

In other words, you may want to fall-thru to case 'p', not case 0
with forced blob 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: [RFC/PATCH 1/4] show: obey --textconv for blobs

2013-02-06 Thread Junio C Hamano
Michael J Gruber g...@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?

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.

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.

 Signed-off-by: Michael J Gruber g...@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;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 0/4] textconv for show and grep

2013-02-06 Thread Junio C Hamano
The parts for grep in the series makes tons of sense to me.  I am
not yet convinced about the other two, though.

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] git-send-email: add ~/.authinfo parsing

2013-02-06 Thread Ted Zlatanov
On Wed, 06 Feb 2013 17:41:01 +0100 Matthieu Moy matthieu@grenoble-inp.fr 
wrote: 

MM Ted Zlatanov t...@lifelogs.com writes:
 - sort the output tokens (after 'url' is extracted) so the output is 
 consistent and testable

MM Why not, if you want to use the output of credential_write in tests. But
MM credential_write is essentially used to talk to git credential, so the
MM important information is the content of the hash before credential_write
MM and after credential_read. They are unordered, but consistent and
MM testable.

I like testing output (especially when it's part of an API), so we
should make the externally observable output consistent and testable.

The change is tiny, just sort the keys instead of calling each(), so I
hope it makes it in the final version.

 Yup.  But what you call read and write are, to the credential
 helper, write and read but it's the same protocol :)  So maybe the
 names should be changed to reflect that, e.g. query and response.

MM I don't think that would be a better naming. Maybe serialize and
MM parse would be better, but query would sound like it establishes the
MM connection and possibly reads the response to me.

I'm OK with anything unambiguous.

Thanks!
Ted
--
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: CodingGuidelines Perl amendment

2013-02-06 Thread demerphq
On 6 February 2013 17:29, Junio C Hamano gits...@pobox.com wrote:
 Ted Zlatanov t...@lifelogs.com writes:

  - As in C (see above), we avoid using braces unnecessarily (but Perl
forces braces around if/unless/else/foreach blocks, so this is not
always possible).

 Is it ever (as opposed to not always) possible to omit braces?

Only in a statement modifier.

 It sounds as if we encourage the use of statement modifiers, which
 certainly is not what I want to see.

As you mention below statement modifiers have their place. For instance

  next if $whatever;

Is considered preferable to

if ($whatever) {
  next;
}

Similarly

open my $fh, , $filename
   or die Failed to open '$filename': $!;

Is considered preferable by most Perl programmers to:

my $fh;
if ( not open $fh, , $filename ) {
  die Failed to open '$filename': $!;
}

 You probably would want to mention that opening braces for
 if/else/elsif do not sit on their own line,
 and closing braces for
 them will be followed the next else/elseif on the same line
 instead, but that is part of most of the C guidelines above apply
 so it may be redundant.

  - Don't abuse statement modifiers (unless $youmust).

 It does not make a useful guidance to leave $youmust part
 unspecified.

 Incidentally, your sentence is a good example of where use of
 statement modifiers is appropriate: $youmust is rarely true.

unless often leads to maintenance errors as the expression gets more
complicated over time, more branches need to be added to the
statement, etc. Basically people are bad at doing De Morgans law in
their head.

 In general:

 ... do something ...
 do_this() unless (condition);
 ... do something else ...

 is easier to follow the flow of the logic than

 ... do something ...
 unless (condition) {
 do_this();
 }
 ... do something else ...

 *only* when condition is extremely rare, iow, when do_this() is
 expected to be almost always called.

if (not $condition) {
  do_this();
}

Is much less error prone in terms of maintenance than

unless ($condition) {
  do_this();
}

Similarly

do_this() if not $condition;

leads to less maintenance errors than

do_this() unless $condition;

So if you objective is maintainability I would just ban unless outright.

Cheers,
Yves

-- 
perl -Mre=debug -e /just|another|perl|hacker/
--
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] Update CodingGuidelines for Perl 5

2013-02-06 Thread Ted Zlatanov
Update the coding guidelines for Perl 5.

Signed-off-by: Ted Zlatanov t...@lifelogs.com
---
 Documentation/CodingGuidelines |   44 
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 1d7de5f..951d74c 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -179,6 +179,50 @@ For C programs:
  - Use Git's gettext wrappers to make the user interface
translatable. See Marking strings for translation in po/README.
 
+For Perl 5 programs:
+
+ - Most of the C guidelines above apply.
+
+ - We try to support Perl 5.8 and later (use Perl 5.008).
+
+ - use strict and use warnings are strongly preferred.
+
+ - As in C (see above), we avoid using braces unnecessarily (but Perl forces
+   braces around if/unless/else/foreach blocks, so this is not always 
possible).
+   At least make sure braces do not sit on their own line, like with C.
+
+ - Don't abuse statement modifiers--they are discouraged.  But in general:
+
+   ... do something ...
+   do_this() unless (condition);
+... do something else ...
+
+   should be used instead of
+
+   ... do something ...
+   unless (condition) {
+   do_this();
+   }
+... do something else ...
+
+   *only* when when the condition is so rare that do_this() will be called
+   almost always.
+
+ - We try to avoid assignments inside if().
+
+ - Learn and use Git.pm if you need that functionality.
+
+ - For Emacs, it's useful to put the following in
+   GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:
+
+;; note the first part is useful for C editing, too
+((nil . ((indent-tabs-mode . t)
+  (tab-width . 8)
+  (fill-column . 80)))
+ (cperl-mode . ((cperl-indent-level . 8)
+(cperl-extra-newline-before-brace . nil)
+(cperl-merge-trailing-else . t
+
 Writing Documentation:
 
  Every user-visible change should be reflected in the documentation.
-- 
1.7.9.rc2

--
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: CodingGuidelines Perl amendment

2013-02-06 Thread Ted Zlatanov
On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano gits...@pobox.com wrote: 

JCH Is it ever (as opposed to not always) possible to omit braces?

Oh yes!  Not that I recommend it, and I'm not even going to touch on
Perl Golf :)

JCH It sounds as if we encourage the use of statement modifiers, which
JCH certainly is not what I want to see.

Yup.  I think I captured that in the patch, but please feel free to
revise it after applying or throw it back to me.

JCH You probably would want to mention that opening braces for
JCH if/else/elsif do not sit on their own line, and closing braces for
JCH them will be followed the next else/elseif on the same line
JCH instead, but that is part of most of the C guidelines above apply
JCH so it may be redundant.

OK; done.

 - Don't abuse statement modifiers (unless $youmust).

JCH It does not make a useful guidance to leave $youmust part
JCH unspecified.

JCH Incidentally, your sentence is a good example of where use of
JCH statement modifiers is appropriate: $youmust is rarely true.

I was trying to be funny, honestly.  But OK; reworded.

Ted
--
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: How to diff 2 file revisions with gitk

2013-02-06 Thread Johannes Sixt
Am 06.02.2013 16:57, schrieb R. Diez:
 I would like to start gitk, select with the mouse 2 
 revisions of some file and then compare them, hopefully with an external
  diff tool, very much like I am used to with WinCVS.
 
 The closest I
  got is to start gitk with a filename as an argument, in order to 
 restrict the log to that one file. Then I right-click on a commit (a 
 file revision) and choose Mark this commit. However, if I right-click 
 on another commit and choose Compare with marked commit, I get a full 
 commit diff with all files, and not just the file I specified on the 
 command-line arguments.

Edit-Preferences, tick 'Limit diff to listed paths'.

-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other directories

2013-02-06 Thread Matthieu Moy
perl.mak uses relative path, which is OK when called from the toplevel,
but won't be anymore if one includes it from elsewhere. It is now
possible to include the file using:

GIT_ROOT_DIR=whatever
include $(GIT_ROOT_DIR)/perl.mak

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 perl.mak | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/perl.mak b/perl.mak
index 8bbeef3..a2b8717 100644
--- a/perl.mak
+++ b/perl.mak
@@ -1,5 +1,9 @@
 # Rules to build Git commands written in perl
 
+ifndef GIT_ROOT_DIR
+   GIT_ROOT_DIR = .
+endif
+
 ifndef PERL_PATH
PERL_PATH = /usr/bin/perl
 endif
@@ -11,12 +15,11 @@ NO_PERL = NoThanks
 endif
 
 ifndef NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
-
+$(patsubst %.perl,%,$(SCRIPT_PERL)): $(GIT_ROOT_DIR)/perl/perl.mak
 
-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl 
$(GIT_ROOT_DIR)/GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+  \
-   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory 
instlibdir`  \
+   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C $(GIT_ROOT_DIR)/perl -s 
--no-print-directory instlibdir`  \
sed -e '1{' \
-e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
-e 'h' \
-- 
1.8.1.2.526.gf51a757

--
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/4] Makefile: extract perl-related rules to make them available from other dirs

2013-02-06 Thread Matthieu Moy
The final goal is to make it easy to write Git commands in perl in the
contrib/ directory. It is currently possible to do so, but without the
benefits of Git's Makefile: adapt first line with $(PERL_PATH),
hardcode the path to Git.pm, ...

We make the perl-related part of the Makefile available from directories
other than the toplevel so that:

* Developers can include it, to avoid code duplication

* Users can get a consistent behavior of make install

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 Makefile | 46 +-
 perl.mak | 49 +
 2 files changed, 50 insertions(+), 45 deletions(-)
 create mode 100644 perl.mak

diff --git a/Makefile b/Makefile
index 731b6a8..f39d4a9 100644
--- a/Makefile
+++ b/Makefile
@@ -573,14 +573,10 @@ BINDIR_PROGRAMS_NO_X += git-cvsserver
 ifndef SHELL_PATH
SHELL_PATH = /bin/sh
 endif
-ifndef PERL_PATH
-   PERL_PATH = /usr/bin/perl
-endif
 ifndef PYTHON_PATH
PYTHON_PATH = /usr/bin/python
 endif
 
-export PERL_PATH
 export PYTHON_PATH
 
 LIB_FILE = libgit.a
@@ -1441,10 +1437,6 @@ ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
 
-ifeq ($(PERL_PATH),)
-NO_PERL = NoThanks
-endif
-
 ifeq ($(PYTHON_PATH),)
 NO_PYTHON = NoThanks
 endif
@@ -1522,7 +1514,6 @@ prefix_SQ = $(subst ','\'',$(prefix))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
@@ -1715,9 +1706,6 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script)  \
mv $@+ $@
 
-ifndef NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
-
 perl/perl.mak: perl/PM.stamp
 
 perl/PM.stamp: FORCE
@@ -1728,39 +1716,7 @@ perl/PM.stamp: FORCE
 perl/perl.mak: GIT-CFLAGS GIT-PREFIX perl/Makefile perl/Makefile.PL
$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' 
prefix='$(prefix_SQ)' $(@F)
 
-$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
-   $(QUIET_GEN)$(RM) $@ $@+  \
-   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory 
instlibdir`  \
-   sed -e '1{' \
-   -e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
-   -e 'h' \
-   -e 's=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || 
'$$INSTLIBDIR'));=' \
-   -e 'H' \
-   -e 'x' \
-   -e '}' \
-   -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
-   $@.perl $@+  \
-   chmod +x $@+  \
-   mv $@+ $@
-
-
-.PHONY: gitweb
-gitweb:
-   $(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
-
-git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES
-   $(QUIET_GEN)$(cmd_munge_script)  \
-   chmod +x $@+  \
-   mv $@+ $@
-else # NO_PERL
-$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
-   $(QUIET_GEN)$(RM) $@ $@+  \
-   sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
-   -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
-   unimplemented.sh $@+  \
-   chmod +x $@+  \
-   mv $@+ $@
-endif # NO_PERL
+include perl.mak
 
 ifndef NO_PYTHON
 $(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
diff --git a/perl.mak b/perl.mak
new file mode 100644
index 000..8bbeef3
--- /dev/null
+++ b/perl.mak
@@ -0,0 +1,49 @@
+# Rules to build Git commands written in perl
+
+ifndef PERL_PATH
+   PERL_PATH = /usr/bin/perl
+endif
+export PERL_PATH
+PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+
+ifeq ($(PERL_PATH),)
+NO_PERL = NoThanks
+endif
+
+ifndef NO_PERL
+$(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak
+
+
+$(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl GIT-VERSION-FILE
+   $(QUIET_GEN)$(RM) $@ $@+  \
+   INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C perl -s --no-print-directory 
instlibdir`  \
+   sed -e '1{' \
+   -e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
+   -e 'h' \
+   -e 's=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} || 
'$$INSTLIBDIR'));=' \
+   -e 'H' \
+   -e 'x' \
+   -e '}' \
+   -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
+   $@.perl $@+  \
+   chmod +x $@+  \
+   mv $@+ $@
+
+
+.PHONY: gitweb
+gitweb:
+   $(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
+
+git-instaweb: git-instaweb.sh gitweb GIT-SCRIPT-DEFINES
+   $(QUIET_GEN)$(cmd_munge_script)  \
+   chmod +x $@+  \
+   mv $@+ $@
+else # NO_PERL
+$(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
+   $(QUIET_GEN)$(RM) $@ $@+  \
+   sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+   -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
+   unimplemented.sh $@+  \
+   chmod +x $@+  \
+   mv $@+ $@
+endif # NO_PERL
-- 
1.8.1.2.526.gf51a757

--
To unsubscribe from this list: send the line 

[PATCH 3/4] Makefile: factor common configuration in git-default-config.mak

2013-02-06 Thread Matthieu Moy
Similarly to the extraction of perl-related code in perl.mak, we extract
general default configuration from the Makefile to make it available from
directories other than the toplevel.

This is required to make perl.mak usable because it requires $(pathsep)
to be set.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 Makefile   | 62 +-
 default-config.mak | 61 +
 2 files changed, 62 insertions(+), 61 deletions(-)
 create mode 100644 default-config.mak

diff --git a/Makefile b/Makefile
index f39d4a9..9649a41 100644
--- a/Makefile
+++ b/Makefile
@@ -346,67 +346,7 @@ GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
 
-# CFLAGS and LDFLAGS are for the users to override from the command line.
-
-CFLAGS = -g -O2 -Wall
-LDFLAGS =
-ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
-ALL_LDFLAGS = $(LDFLAGS)
-STRIP ?= strip
-
-# Among the variables below, these:
-#   gitexecdir
-#   template_dir
-#   mandir
-#   infodir
-#   htmldir
-#   sysconfdir
-# can be specified as a relative path some/where/else;
-# this is interpreted as relative to $(prefix) and git at
-# runtime figures out where they are based on the path to the executable.
-# This can help installing the suite in a relocatable way.
-
-prefix = $(HOME)
-bindir_relative = bin
-bindir = $(prefix)/$(bindir_relative)
-mandir = share/man
-infodir = share/info
-gitexecdir = libexec/git-core
-mergetoolsdir = $(gitexecdir)/mergetools
-sharedir = $(prefix)/share
-gitwebdir = $(sharedir)/gitweb
-localedir = $(sharedir)/locale
-template_dir = share/git-core/templates
-htmldir = share/doc/git-doc
-ETC_GITCONFIG = $(sysconfdir)/gitconfig
-ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
-lib = lib
-# DESTDIR =
-pathsep = :
-
-export prefix bindir sharedir sysconfdir gitwebdir localedir
-
-CC = cc
-AR = ar
-RM = rm -f
-DIFF = diff
-TAR = tar
-FIND = find
-INSTALL = install
-RPMBUILD = rpmbuild
-TCL_PATH = tclsh
-TCLTK_PATH = wish
-XGETTEXT = xgettext
-MSGFMT = msgfmt
-PTHREAD_LIBS = -lpthread
-PTHREAD_CFLAGS =
-GCOV = gcov
-
-export TCL_PATH TCLTK_PATH
-
-SPARSE_FLAGS =
-
-
+include default-config.mak
 
 ### --- END CONFIGURATION SECTION ---
 
diff --git a/default-config.mak b/default-config.mak
new file mode 100644
index 000..b2aab3d
--- /dev/null
+++ b/default-config.mak
@@ -0,0 +1,61 @@
+# CFLAGS and LDFLAGS are for the users to override from the command line.
+
+CFLAGS = -g -O2 -Wall
+LDFLAGS =
+ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
+ALL_LDFLAGS = $(LDFLAGS)
+STRIP ?= strip
+
+# Among the variables below, these:
+#   gitexecdir
+#   template_dir
+#   mandir
+#   infodir
+#   htmldir
+#   sysconfdir
+# can be specified as a relative path some/where/else;
+# this is interpreted as relative to $(prefix) and git at
+# runtime figures out where they are based on the path to the executable.
+# This can help installing the suite in a relocatable way.
+
+prefix = $(HOME)
+bindir_relative = bin
+bindir = $(prefix)/$(bindir_relative)
+mandir = share/man
+infodir = share/info
+gitexecdir = libexec/git-core
+mergetoolsdir = $(gitexecdir)/mergetools
+sharedir = $(prefix)/share
+gitwebdir = $(sharedir)/gitweb
+localedir = $(sharedir)/locale
+template_dir = share/git-core/templates
+htmldir = share/doc/git-doc
+ETC_GITCONFIG = $(sysconfdir)/gitconfig
+ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
+lib = lib
+# DESTDIR =
+pathsep = :
+
+export prefix bindir sharedir sysconfdir gitwebdir localedir
+
+CC = cc
+AR = ar
+RM = rm -f
+DIFF = diff
+TAR = tar
+FIND = find
+INSTALL = install
+RPMBUILD = rpmbuild
+TCL_PATH = tclsh
+TCLTK_PATH = wish
+XGETTEXT = xgettext
+MSGFMT = msgfmt
+PTHREAD_LIBS = -lpthread
+PTHREAD_CFLAGS =
+GCOV = gcov
+
+export TCL_PATH TCLTK_PATH
+
+SPARSE_FLAGS =
+
+
-- 
1.8.1.2.526.gf51a757

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/4] Allow contrib/ to use Git's Makefile for perl code

2013-02-06 Thread Matthieu Moy
The very final goal is to be able to move painlessly (credential) code
from git-remote-mediawiki to Git.pm, but then it's nice for the user
to be able to say just cd contrib/mw-to-git  make install and let
the Makefile set perl's library path just like other Git commands
written in perl.

This series does this while trying to minimize code duplication, and
to make it easy for future other tools in contrib to do the same.

Matthieu Moy (4):
  Makefile: extract perl-related rules to make them available from other
dirs
  perl.mak: introduce $(GIT_ROOT_DIR) to allow inclusion from other
directories
  Makefile: factor common configuration in git-default-config.mak
  git-remote-mediawiki: use Git's Makefile to build the script

 Makefile   | 108 +
 contrib/mw-to-git/.gitignore   |   1 +
 contrib/mw-to-git/Makefile |  45 ++---
 ...-remote-mediawiki = git-remote-mediawiki.perl} |   0
 default-config.mak |  61 
 perl.mak   |  52 ++
 6 files changed, 145 insertions(+), 122 deletions(-)
 create mode 100644 contrib/mw-to-git/.gitignore
 rename contrib/mw-to-git/{git-remote-mediawiki = git-remote-mediawiki.perl} 
(100%)
 create mode 100644 default-config.mak
 create mode 100644 perl.mak

-- 
1.8.1.2.526.gf51a757

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] git-remote-mediawiki: use Git's Makefile to build the script

2013-02-06 Thread Matthieu Moy
The configuration of the install directory is not reused from the
toplevel Makefile: we assume Git is already built, hence just call
git --exec-path. This avoids too much surgery in the toplevel Makefile.

git-remote-mediawiki.perl can now use Git;.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 contrib/mw-to-git/.gitignore   |  1 +
 contrib/mw-to-git/Makefile | 45 ++
 ...-remote-mediawiki = git-remote-mediawiki.perl} |  0
 3 files changed, 30 insertions(+), 16 deletions(-)
 create mode 100644 contrib/mw-to-git/.gitignore
 rename contrib/mw-to-git/{git-remote-mediawiki = git-remote-mediawiki.perl} 
(100%)

diff --git a/contrib/mw-to-git/.gitignore b/contrib/mw-to-git/.gitignore
new file mode 100644
index 000..b919655
--- /dev/null
+++ b/contrib/mw-to-git/.gitignore
@@ -0,0 +1 @@
+git-remote-mediawiki
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index 3ed728b..ed8073b 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -8,40 +8,53 @@
 #
 ## Build git-remote-mediawiki
 
--include ../../config.mak.autogen
--include ../../config.mak
+all:
+
+GIT_ROOT_DIR=../../
+include $(GIT_ROOT_DIR)/default-config.mak
+-include $(GIT_ROOT_DIR)/config.mak.autogen
+-include $(GIT_ROOT_DIR)/config.mak
+-include $(GIT_ROOT_DIR)/GIT-VERSION-FILE
+
+
+SCRIPT_PERL = git-remote-mediawiki.perl
+ALL_PROGRAMS = $(patsubst %.perl,%,$(SCRIPT_PERL))
+
+include $(GIT_ROOT_DIR)/perl.mak
 
-ifndef PERL_PATH
-   PERL_PATH = /usr/bin/perl
-endif
 ifndef gitexecdir
gitexecdir = $(shell git --exec-path)
 endif
 
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
-gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
-SCRIPT = git-remote-mediawiki
+ifneq ($(filter /%,$(firstword $(gitexecdir))),)
+gitexec_instdir = $(gitexecdir)
+else
+gitexec_instdir = $(prefix)/$(gitexecdir)
+endif
+gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
 
 .PHONY: install help doc test clean
 
 help:
@echo 'This is the help target of the Makefile. Current configuration:'
-   @echo '  gitexecdir = $(gitexecdir_SQ)'
+   @echo '  gitexec_instdir = $(gitexec_instdir_SQ)'
@echo '  PERL_PATH = $(PERL_PATH_SQ)'
-   @echo 'Run $(MAKE) install to install $(SCRIPT) in gitexecdir'
+   @echo 'Run $(MAKE) all to build the script'
+   @echo 'Run $(MAKE) install to install $(ALL_PROGRAMS) in 
gitexec_instdir'
@echo 'Run $(MAKE) test to run the testsuite'
 
-install:
-   sed -e '1s|#!.*/perl|#!$(PERL_PATH_SQ)|' $(SCRIPT) \
-'$(gitexecdir_SQ)/$(SCRIPT)'
-   chmod +x '$(gitexecdir)/$(SCRIPT)'
+all: $(ALL_PROGRAMS)
+
+install: $(ALL_PROGRAMS)
+   $(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 
 doc:
-   @echo 'Sorry, make doc is not implemented yet for $(SCRIPT)'
+   @echo 'Sorry, make doc is not implemented yet for $(ALL_PROGRAMS)'
 
 test:
$(MAKE) -C t/ test
 
 clean:
-   $(RM) '$(gitexecdir)/$(SCRIPT)'
+   $(RM) $(ALL_PROGRAMS)
+   $(RM) $(patsubst %,$(gitexec_instdir)/%,/$(ALL_PROGRAMS))
$(MAKE) -C t/ clean
diff --git a/contrib/mw-to-git/git-remote-mediawiki 
b/contrib/mw-to-git/git-remote-mediawiki.perl
similarity index 100%
rename from contrib/mw-to-git/git-remote-mediawiki
rename to contrib/mw-to-git/git-remote-mediawiki.perl
-- 
1.8.1.2.526.gf51a757

--
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: CodingGuidelines Perl amendment

2013-02-06 Thread Junio C Hamano
demerphq demer...@gmail.com writes:

 As you mention below statement modifiers have their place. For instance

   next if $whatever;

 Is considered preferable to

 if ($whatever) {
   next;
 }

 Similarly

 open my $fh, , $filename
or die Failed to open '$filename': $!;

 Is considered preferable by most Perl programmers to:

 my $fh;
 if ( not open $fh, , $filename ) {
   die Failed to open '$filename': $!;
 }

Yeah, and that is for the same reason.  When you are trying to get a
birds-eye view of the codeflow, the former makes it clear that we
do something, and then we open, and then we ..., without letting
the error handling (which also is rare case) distract us.

 unless often leads to maintenance errors as the expression gets more
 complicated over time,...

That might also be true, but my comment was not an endorsement for
(or suggestion against) use of unless.  I was commenting on
statement modifiers, which some people tend to overuse (or abuse)
and make the resulting code harder to follow.
--
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: CodingGuidelines Perl amendment

2013-02-06 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano gits...@pobox.com wrote: 

 JCH Is it ever (as opposed to not always) possible to omit braces?

 Oh yes!  Not that I recommend it, and I'm not even going to touch on
 Perl Golf :)

 JCH It sounds as if we encourage the use of statement modifiers, which
 JCH certainly is not what I want to see.

 Yup.  I think I captured that in the patch, but please feel free to
 revise it after applying or throw it back to me.

I'd suggest to just drop that try to write without braces entirely.

 JCH Incidentally, your sentence is a good example of where use of
 JCH statement modifiers is appropriate: $youmust is rarely true.

 I was trying to be funny, honestly.  But OK; reworded.

It wasn't a useful guidance, but it _was_ funny.  
--
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: CodingGuidelines Perl amendment

2013-02-06 Thread demerphq
On 6 February 2013 19:14, Junio C Hamano gits...@pobox.com wrote:
 demerphq demer...@gmail.com writes:

 As you mention below statement modifiers have their place. For instance

   next if $whatever;

 Is considered preferable to

 if ($whatever) {
   next;
 }

 Similarly

 open my $fh, , $filename
or die Failed to open '$filename': $!;

 Is considered preferable by most Perl programmers to:

 my $fh;
 if ( not open $fh, , $filename ) {
   die Failed to open '$filename': $!;
 }

 Yeah, and that is for the same reason.  When you are trying to get a
 birds-eye view of the codeflow, the former makes it clear that we
 do something, and then we open, and then we ..., without letting
 the error handling (which also is rare case) distract us.

perldoc perlstyle has language which explains this well if you want to
crib a description from somewhere.

 unless often leads to maintenance errors as the expression gets more
 complicated over time,...

 That might also be true, but my comment was not an endorsement for
 (or suggestion against) use of unless.  I was commenting on
 statement modifiers, which some people tend to overuse (or abuse)
 and make the resulting code harder to follow.

That's also my point about unless. They tend to get abused and then
lead to maint devs making errors, and people misunderstanding the
code. The only time that unless IMO is ok (ish) is when it really is
a very simple statement. As soon as it mentions more than one var it
should be converted to an if. This applies even more so to the
modifier form.

Yves

-- 
perl -Mre=debug -e /just|another|perl|hacker/
--
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: CodingGuidelines Perl amendment

2013-02-06 Thread Ted Zlatanov
On Wed, 06 Feb 2013 10:16:21 -0800 Junio C Hamano gits...@pobox.com wrote: 

JCH I'd suggest to just drop that try to write without braces entirely.

OK, I'll do it on the reroll, or you can just make the change directly.

I agree it was not going anywhere :)

Ted

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 951d74c..857f4e2 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -187,10 +187,6 @@ For Perl 5 programs:
 
  - use strict and use warnings are strongly preferred.
 
- - As in C (see above), we avoid using braces unnecessarily (but Perl forces
-   braces around if/unless/else/foreach blocks, so this is not always 
possible).
-   At least make sure braces do not sit on their own line, like with C.
-
  - Don't abuse statement modifiers--they are discouraged.  But in general:
 
... do 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


What's cooking in git.git (Feb 2013, #03; Wed, 6)

2013-02-06 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

As usual, this cycle is expected to last for 8 to 10 weeks, with a
preview -rc0 sometime in the middle of this month.

You can find the changes described here in the integration branches of the
repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to master]

* ft/transport-report-segv (2013-01-31) 1 commit
  (merged to 'next' on 2013-02-02 at 6c450a7)
 + push: fix segfault when HEAD points nowhere

 A failure to push due to non-ff while on an unborn branch
 dereferenced a NULL pointer when showing an error message.


* jc/fake-ancestor-with-non-blobs (2013-01-31) 3 commits
  (merged to 'next' on 2013-02-02 at 86d457a)
 + apply: diagnose incomplete submodule object name better
 + apply: simplify build_fake_ancestor()
 + git-am: record full index line in the patch used while rebasing
 (this branch is used by jc/extended-fake-ancestor-for-gitlink.)

 Rebasing the history of superproject with change in the submodule
 has been broken since v1.7.12.


* jn/auto-depend-workaround-buggy-ccache (2013-02-01) 1 commit
  (merged to 'next' on 2013-02-02 at db5940a)
 + Makefile: explicitly set target name for autogenerated dependencies

 An age-old workaround to prevent buggy versions of ccache from
 breaking the auto-generation of dependencies, which unfortunately
 is still relevant because some people use ancient distros.


* sb/gpg-plug-fd-leak (2013-01-31) 1 commit
  (merged to 'next' on 2013-02-02 at c271a31)
 + gpg: close stderr once finished with it in verify_signed_buffer()

 We forgot to close the file descriptor reading from gpg output,
 killing git log --show-signature on a long history.


* ta/doc-no-small-caps (2013-02-01) 6 commits
  (merged to 'next' on 2013-02-02 at 77cbd0e)
 + Documentation: StGit is the right spelling, not StGIT
 + Documentation: describe the repository in repository-layout
 + Documentation: add a description for 'gitfile' to glossary
 + Documentation: do not use undefined terms git-dir and git-file
 + Documentation: the name of the system is 'Git', not 'git'
 + Documentation: avoid poor-man's small caps GIT

 Update documentation to change GIT which was a poor-man's small
 caps to Git.  The latter was the intended spelling.

 Also change git spelled in all-lowercase to Git when it refers
 to the system as the whole or the concept it embodies, as opposed to
 the command the end users would type.

--
[New Topics]

* jc/extended-fake-ancestor-for-gitlink (2013-02-05) 1 commit
 - apply: verify submodule commit object name better

 Instead of requiring the full 40-hex object names on the index
 line, we can read submodule commit object names from the textual
 diff when synthesizing a fake ancestore tree for git am -3.

 Will merge to 'next'.


* tz/credential-authinfo (2013-02-05) 1 commit
 - Add contrib/credentials/netrc with GPG support

 A new read-only credential helper (in contrib/) to interact with
 the .netrc/.authinfo files.  Hopefully mn/send-email-authinfo topic
 can rebuild on top of something like this.

 Waiting for further reviews.


* jx/utf8-printf-width (2013-02-05) 1 commit
 - Add utf8_fprintf helper which returns correct columns

 Use a new helper that prints a message and counts its display width
 to align the help messages parse-options produces.

--
[Stalled]

* mn/send-email-authinfo (2013-01-29) 1 commit
 - git-send-email: add ~/.authinfo parsing

 This triggered a subtopic to add a credential helper for
 authinfo/netrc files (with or without GPG encryption), but nobody
 seems to be working on connecting send-email to the credential
 framework.


* mp/diff-algo-config (2013-01-16) 3 commits
 - diff: Introduce --diff-algorithm command line option
 - config: Introduce diff.algorithm variable
 - git-completion.bash: Autocomplete --minimal and --histogram for git-diff

 Add diff.algorithm configuration so that the user does not type
 diff --histogram.

 Looking better; may want tests to protect it from future breakages,
 but otherwise it looks ready for 'next'.

 Expecting a follow-up to add tests.


* mb/gitweb-highlight-link-target (2012-12-20) 1 commit
 - Highlight the link target line in Gitweb using CSS

 Expecting a reroll.
 $gmane/211935


* jk/lua-hackery (2012-10-07) 6 commits
 - pretty: fix up one-off format_commit_message calls
 - Minimum compilation fixup
 - Makefile: make lua a bit more configurable
 - add a lua pretty format
 - add basic lua infrastructure
 - pretty: make some commit-parsing helpers more public

 Interesting exercise. When we do this for real, we probably would want
 to wrap a commit to make it more like an object with methods like
 parents, etc.


* rc/maint-complete-git-p4 (2012-09-24) 1 

Re: Bug in git log --graph -p -m (version 1.7.7.6)

2013-02-06 Thread Matthieu Moy
John Keeping j...@keeping.me.uk writes:

 I would argue that the line should start with | | , since it really is
 just a continuation of the same commit.

 | | 
 | | commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 
 33e70e70c0173d634826b998bdc304f93c0966b8)
 | | Merge: 2c1e6a3 33e70e7
 | | Author: Matthieu Moy matthieu@imag.fr
 | | Date:   Tue Feb 5 22:05:33 2013 +0100

Yes.

I had a look at the code, I guess the call to graph_show_commit() in
show_log() (in log-tree.c) should have called graph_show_padding() but
didn't in this case. Then I got lost in graph.c :-(.

-- 
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: CodingGuidelines Perl amendment

2013-02-06 Thread Ted Zlatanov
On Wed, 6 Feb 2013 19:25:43 +0100 demerphq demer...@gmail.com wrote: 

d On 6 February 2013 19:05, Ted Zlatanov t...@lifelogs.com wrote:
 On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano gits...@pobox.com wrote:
 
JCH Is it ever (as opposed to not always) possible to omit braces?
 
 Oh yes!  Not that I recommend it, and I'm not even going to touch on
 Perl Golf :)

d I think you are wrong. Can you provide an example?

d Larry specifically wanted to avoid the dangling else problem that C
d suffers from, and made it so that blocks are mandatory. The only
d exception is statement modifiers, which are not only allowed to omit
d the braces but also the parens on the condition.

Oh, perhaps I didn't state it correctly.  You can avoid braces, but not
if you want to use if/elsif/else/unless/etc. which require them:

condition  do_this();
condition || do_this();
condition ? do_this() : do_that();

(and others I can't recall right now)

But my point was only that it's always possible to get around these
artificial restrictions; it's more important to ask for legible sensible
code.  Sorry if that was unclear!

Ted
--
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: CodingGuidelines Perl amendment

2013-02-06 Thread demerphq
On 6 February 2013 19:35, Ted Zlatanov t...@lifelogs.com wrote:
 On Wed, 6 Feb 2013 19:25:43 +0100 demerphq demer...@gmail.com wrote:

 d On 6 February 2013 19:05, Ted Zlatanov t...@lifelogs.com wrote:
 On Wed, 06 Feb 2013 08:29:30 -0800 Junio C Hamano gits...@pobox.com wrote:

 JCH Is it ever (as opposed to not always) possible to omit braces?

 Oh yes!  Not that I recommend it, and I'm not even going to touch on
 Perl Golf :)

 d I think you are wrong. Can you provide an example?

 d Larry specifically wanted to avoid the dangling else problem that C
 d suffers from, and made it so that blocks are mandatory. The only
 d exception is statement modifiers, which are not only allowed to omit
 d the braces but also the parens on the condition.

 Oh, perhaps I didn't state it correctly.  You can avoid braces, but not
 if you want to use if/elsif/else/unless/etc. which require them:

 condition  do_this();
 condition || do_this();
 condition ? do_this() : do_that();

 (and others I can't recall right now)

 But my point was only that it's always possible to get around these
 artificial restrictions; it's more important to ask for legible sensible
 code.  Sorry if that was unclear!

Ah ok. Right, at a low level:

if (condition) { do_this() }

is identical to

condition  do_this();

IOW, Perl allows logical operators to act as control flow statements.

I hope your document include something that says that using logical
operators as control flow statements should be used sparingly, and
generally should be restricted to low precedence operators and should
never involve more than one operator.

Yves




-- 
perl -Mre=debug -e /just|another|perl|hacker/
--
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: CodingGuidelines Perl amendment

2013-02-06 Thread Ted Zlatanov
On Wed, 6 Feb 2013 19:44:16 +0100 demerphq demer...@gmail.com wrote: 

d Ah ok. Right, at a low level:

d if (condition) { do_this() }

d is identical to

d condition  do_this();

d IOW, Perl allows logical operators to act as control flow statements.

d I hope your document include something that says that using logical
d operators as control flow statements should be used sparingly, and
d generally should be restricted to low precedence operators and should
d never involve more than one operator.

I'd stay away from wording it so tightly, but instead just say

Make your code readable and sensible, and don't try to be clever.

But this is good C and shell advice too, so I'd put it under General
Guidelines and leave it for Junio to decide if it's appropriate.

Ted
--
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/8] Hiding refs

2013-02-06 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 Hiderefs creates a dark corner of a remote git repo that can hold
 arbitrary content that is impossible for anybody to discover but
 nevertheless possible for anybody to download (if they know the name of
 a hidden reference).  In earlier versions of the patch series I believe
 that it was possible to push to a hidden reference hierarchy, which made
 it possible to upload dark content.  The new version appears (from the
 code) to prohibit adding references in a hidden hierarchy, which would
 close the main loophole that I was worried about.  But the documentation
 and the unit tests only explicitly say that updates and deletes are
 prohibited; nothing is said about adding references (unless update is
 understood to include add).  I think the true behavior should be
 clarified and tested.

 I was worried that somehow this dark content could be used for
 malicious purposes; for example, pushing compromised code then
 convincing somebody to download it by SHA1 with the implicit argument
 it's safe since it comes directly from the project's official
 repository.  If it is indeed impossible to populate the dark namespace
 remotely then I can't think of a way to exploit it.

 Or you can think hiderefs is the first step to addressing the
 initial ref advertisment problem.  The series says hidden refs are
 to be fetched out of band, but that's not the only way.

Let me help unconfuse this thread.

I think the series as 8-patch series was poorly presented, and
separating it into two will help understanding what they are about.

The first three:

  upload-pack: share more code
  upload-pack: simplify request validation
  upload/receive-pack: allow hiding ref hierarchies

is _the_ topic of the series.  As far as I am concerned (I am not
speaking for Gerrit users, but am speaking as the Git maintainer),
the topic is solely about uncluttering.  There may be refs that the
server end may need to keep for its operation, but that remote users
have _no_ business knowing about.  Allowing the server to keep these
refs in the repository, while not showing these refs over the wire,
is the problem the series solves.

In other words, it is not about these are *usually* not wanted by
clients, so do not show them by default.  It is about these are
not to be shown, ever.

OK?

Now, there may be some refs that are not *usually* wanted by clients
but there may be cases where clients want to

 (1) learn about them via the same protocol; and/or
 (2) fetch them over the protocol.

If you want to solve both of these two issues generally, the
solution has to involve a separate protocol from the today's
protocol.  It would go like this:

 * The upload-pack-2 service sits on a port different from today's,
   waits for a ls-remote/fetch/clone client to connect to it, makes
   a default advertisement that only includes the refs that are
   usually wanted by clients with hints on what other refs the
   initial advertisement omitted, to let the client know that it is
   allowed to ask for them.

 * An updated client, if it sees that some refs are omitted from the
   initial advertisement *and* what the user told it to fetch or
   list may be one of the omitted ones (this is why the server gives
   hints in the previous step in the first step; when the server
   says it did not omit anything, or when it says it omitted only
   refs/pull/*, a client that wanted to fetch refs/heads/frotz will
   know the request will fail without continuing this step), then
   makes a expand-refs request to the server, asking for the refs
   it did not see and the server could supply.

 * When the server sees expand-refs, it responds with additional
   advertisement.  expand-refs refs/pull/* may result in listing
   of all refs in that hierarchy.  expand-refs refs/changes/1/1
   would result in listing that single ref.  expand-refs no-such
   may result in nothing, indicating an error.

 * After the (possible) expand-refs exchange, the client knows
   exactly the same and necessary information as the current
   protocol gives it in order to go to the common ancestor discovery
   step, and the protocol can continue the same way as the current
   protocol.

Note that this cannot sit on the current port in general, as
existing clients will not be able to tell some refs are not
advertised, so unless you are hiding large and truly unused part of
the refspace, interoperability with older clients will render the
mechanism useless.  You cannot use this to delay the refs/tags/
hierarchy with this mechanism and have older client come to the
updated service that by default does not advertise tags, for
example.

The above is what I called the delayed advertisement in the
discussion, which was brought up several months ago but nothing
materialized as the result.  People who are interested in pursuing
this can volunteer and start discussing the design refinements 

Re: What's cooking in git.git (Feb 2013, #03; Wed, 6)

2013-02-06 Thread Jens Lehmann
Am 06.02.2013 19:29, schrieb Junio C Hamano:
 * jl/submodule-deinit (2013-02-04) 1 commit
  - submodule: add 'deinit' command
 
  There was no Porcelain way to say I no longer am interested in
  this submodule, once you express your interest in a submodule with
  submodule init.  submodule deinit is the way to do so.
 
  Will merge to 'next'.

Oops, I though you were waiting for a reroll. Currently I'm having the
appended interdiff compared to your version. Changes are:

- Add deinit to the --force documentation of git submodule
- Never remove submodules containing a .git dir, even when forced
- diagnostic output when rm -rf or mkdir fails
- More test cases

And I wanted to add three more test cases for modified submodules before
sending v4. You could squash in the first two hunks into the commit you
have in pu and I'll send a follow up patch with the extra tests soon or
you could wait for me sending an updated patch. What do you think?

---8--
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 7a149eb..45ee12b 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -227,8 +227,10 @@ OPTIONS

 -f::
 --force::
-   This option is only valid for add and update commands.
+   This option is only valid for add, deinit and update commands.
When running add, allow adding an otherwise ignored submodule path.
+   When running deinit the submodule work trees will be removed even if
+   they contain local changes.
When running update, throw away local changes in submodules when
switching to a different commit; and always run a checkout operation
in the submodule, even if the commit listed in the index of the
diff --git a/git-submodule.sh b/git-submodule.sh
index f05b597..365c6de 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -595,14 +595,25 @@ cmd_deinit()
continue
fi

-   # Remove the submodule work tree
-   if test -z $force
+   # Remove the submodule work tree (unless the user already did 
it)
+   if test -d $sm_path
then
-   git rm -n $sm_path ||
-   die $(eval_gettext Submodule work tree $sm_path 
contains local modifications, use '-f' to discard them)
+   # Protect submodules containing a .git directory
+   if test -d $sm_path/.git
+   then
+   echo 2 $(eval_gettext Submodule work tree 
$sm_path contains a .git directory)
+   die $(eval_gettext (use 'rm -rf' if you 
really want to remove it including all of its history))
+   fi
+
+   if test -z $force
+   then
+   git rm -n $sm_path ||
+   die $(eval_gettext Submodule work tree 
$sm_path contains local modifications, use '-f' to discard them)
+   fi
+   rm -rf $sm_path || say $(eval_gettext Could not 
remove submodule work tree '\$sm_path')
fi
-   rm -rf $sm_path
-   mkdir $sm_path
+
+   mkdir $sm_path || say $(eval_gettext Could not create empty 
submodule directory '\$sm_path')

# Remove the whole section so we have a clean state when the
# user later decides to init this submodule again
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 34d8274..0567f1a 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -757,20 +757,46 @@ test_expect_success 'submodule add with an existing name 
fails unless forced' '
)
 '

+test_expect_success 'set up a second submodule' '
+   git submodule add ./init2 example2 
+   git commit -m submodle example2 added
+'
+
 test_expect_success 'submodule deinit should remove the whole submodule 
section from .git/config' '
git config submodule.example.foo bar 
+   git config submodule.example2.frotz nitfol 
git submodule deinit init 
test -z $(git config submodule.example.url) 
-   test -z $(git config submodule.example.foo)
+   test -z $(git config submodule.example.foo) 
+   test -n $(git config submodule.example2.url) 
+   test -n $(git config submodule.example2.frotz) 
+   rmdir init
 '

 test_expect_success 'submodule deinit . deinits all initialized submodules' '
git submodule update --init 
git config submodule.example.foo bar 
+   git config submodule.example2.frotz nitfol 
test_must_fail git submodule deinit 
git submodule deinit . 
test -z $(git config submodule.example.url) 
-   test -z $(git config submodule.example.foo)
+   test -z $(git config submodule.example.foo) 
+   test -z $(git config submodule.example2.url) 
+  

Re: CodingGuidelines Perl amendment

2013-02-06 Thread Junio C Hamano
Ted Zlatanov t...@lifelogs.com writes:

 Make your code readable and sensible, and don't try to be clever.

 But this is good C and shell advice too,...

Sounds sensible.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Feb 2013, #03; Wed, 6)

2013-02-06 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 Am 06.02.2013 19:29, schrieb Junio C Hamano:
 * jl/submodule-deinit (2013-02-04) 1 commit
  - submodule: add 'deinit' command
 
  There was no Porcelain way to say I no longer am interested in
  this submodule, once you express your interest in a submodule with
  submodule init.  submodule deinit is the way to do so.
 
  Will merge to 'next'.

 Oops, I though you were waiting for a reroll. Currently I'm having the
 appended interdiff compared to your version. Changes are:

 - Add deinit to the --force documentation of git submodule
 - Never remove submodules containing a .git dir, even when forced
 - diagnostic output when rm -rf or mkdir fails
 - More test cases

 And I wanted to add three more test cases for modified submodules before
 sending v4. You could squash in the first two hunks into the commit you
 have in pu and I'll send a follow up patch with the extra tests soon or
 you could wait for me sending an updated patch. What do you think?

I haven't merged it down to 'next' yet.  So please proceed as you
planned.  Thanks for stopping me.
--
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/8] Hiding refs

2013-02-06 Thread Jonathan Nieder
Junio C Hamano wrote:
 Duy Nguyen pclo...@gmail.com writes:
 On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:

 Hiderefs creates a dark corner of a remote git repo
[...]
 Or you can think hiderefs is the first step to addressing the
 initial ref advertisment problem.  The series says hidden refs are
 to be fetched out of band, but that's not the only way.

 Let me help unconfuse this thread.

 I think the series as 8-patch series was poorly presented, and
 separating it into two will help understanding what they are about.

 The first three:

   upload-pack: share more code
   upload-pack: simplify request validation
   upload/receive-pack: allow hiding ref hierarchies

 is _the_ topic of the series.  As far as I am concerned (I am not
 speaking for Gerrit users, but am speaking as the Git maintainer),
 the topic is solely about uncluttering.  There may be refs that the
 server end may need to keep for its operation, but that remote users
 have _no_ business knowing about.

An obvious question when looking at that alone is, is there ever
actually need for such private refs?  If the refs are not meant to be
shared with users *at all*, why are they even refs?

An answer is because refs force gc to keep the corresponding
objects.  For example, the sysadmin may want to keep refs/archived/
refs for dead branches that should not be advertised or accessible to
the user any more.  Seems sane, though not especially exciting.

What is more exciting to me is that it is a first step toward
addressing the complicated problem of offering access to more refs
than can be efficiently presented in the current ref advertisement.  I
think that's a harder problem but something like this would be needed
in order to support existing clients without performance degredation.

And in the meantime, it helps with the refs/archived case.

Thanks for explaining.
Jonathan
--
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 v2] Update CodingGuidelines for Perl 5

2013-02-06 Thread Ted Zlatanov
Update the coding guidelines for Perl 5.

Signed-off-by: Ted Zlatanov t...@lifelogs.com
---
Changes since PATCHv1:
- removed brace guidelines
- add don't try to be clever at beginning

 Documentation/CodingGuidelines |   42 
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 1d7de5f..166c141 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -18,6 +18,8 @@ code.  For Git in general, three rough rules are:
judgement call, the decision based more on real world
constraints people face than what the paper standard says.
 
+For any programming language below, make your code readable and sensible, and
+don't try to be clever.
 
 As for more concrete guidelines, just imitate the existing code
 (this is a good guideline, no matter which project you are
@@ -179,6 +181,46 @@ For C programs:
  - Use Git's gettext wrappers to make the user interface
translatable. See Marking strings for translation in po/README.
 
+For Perl 5 programs:
+
+ - Most of the C guidelines above apply.
+
+ - We try to support Perl 5.8 and later (use Perl 5.008).
+
+ - use strict and use warnings are strongly preferred.
+
+ - Don't abuse statement modifiers--they are discouraged.  But in general:
+
+   ... do something ...
+   do_this() unless (condition);
+... do something else ...
+
+   should be used instead of
+
+   ... do something ...
+   unless (condition) {
+   do_this();
+   }
+... do something else ...
+
+   *only* when when the condition is so rare that do_this() will be called
+   almost always.
+
+ - We try to avoid assignments inside if().
+
+ - Learn and use Git.pm if you need that functionality.
+
+ - For Emacs, it's useful to put the following in
+   GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:
+
+;; note the first part is useful for C editing, too
+((nil . ((indent-tabs-mode . t)
+  (tab-width . 8)
+  (fill-column . 80)))
+ (cperl-mode . ((cperl-indent-level . 8)
+(cperl-extra-newline-before-brace . nil)
+(cperl-merge-trailing-else . t
+
 Writing Documentation:
 
  Every user-visible change should be reflected in the documentation.
-- 
1.7.9.rc2

--
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/8] Hiding refs

2013-02-06 Thread Jonathan Nieder
Michael Haggerty wrote:

 Scenario 1: Some providers junk up their users' repositories with
 content that is not created by the repository's owner and that the owner
 doesn't want to appear to vouch for (e.g., GitHub pull requests).  These
 references might sometimes be useful to fetch, singly or in bulk.

 Scenario 2: Some systems junk up their users' repositories with
 additional references that are not interesting to most pullers (e.g.,
 Gerrit activity markers) though they don't add questionable content.

Actually Gerrit's refs/changes refs are pretty similar to Github's
refs/pull.  Both are requests for code review.

[...]
 But now every time I do a gitk --all or git log --decorate, the
 output is cluttered with all of his references (most of which are just
 old versions of references from the upstream repository that we both
 use).  I would like to be able to hide his references most of the time
 but turn them back on when I need them.

 Scenario 5: Our upstream repository has gazillions of release tags under
 refs/tags/releases/..., sometimes including customer-specific
 releases.  In my daily life these are just clutter.

For both of these use cases, putting the refs somewhere other than
refs/heads, refs/tags, and refs/remotes should be enough to avoid
clutter.

I agree that a --decorate-glob along the lines of git rev-parse's
--glob would be nice.

[...]
 * Some small improvements (e.g. allowing *multiple* views to be
   defined) would provide much more benefit for about the same effort,
   and would be a better base for building other features in the future
   (e.g., local views).

Would advertising GIT_CONFIG_PARAMETERS and giving examples for server
admins to set it in inetd et al to provide different kinds of access
to a same repository through different URLs work?

 Thanks for listening.
 Michael

 [1] Theoretically one could support multiple views of a single
 repository by using something like GIT_CONFIG=view_1_config git
 upload-pack ... or git -c transfer.hiderefs=... git upload-pack ...,
 but this would be awkward.

Ah, I missed this comment before.  What's awkward about that?  I
think it's a clean way to make many aspects of how a repository is
presented (including hook actions) configurable.

Thanks for your help clarifying this feature.  Hopefully some of the
discussion will filter into the documentation.

Jonathan
--
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] graph: output padding for merge subsequent parents

2013-02-06 Thread John Keeping
On Wed, Feb 06, 2013 at 07:33:08PM +0100, Matthieu Moy wrote:
 John Keeping j...@keeping.me.uk writes:
 
  I would argue that the line should start with | | , since it really is
  just a continuation of the same commit.
 
  | | 
  | | commit a393ed598e9fb11436f85bd58f1a38c82f2cadb7 (from 
  33e70e70c0173d634826b998bdc304f93c0966b8)
  | | Merge: 2c1e6a3 33e70e7
  | | Author: Matthieu Moy matthieu@imag.fr
  | | Date:   Tue Feb 5 22:05:33 2013 +0100
 
 Yes.
 
 I had a look at the code, I guess the call to graph_show_commit() in
 show_log() (in log-tree.c) should have called graph_show_padding() but
 didn't in this case. Then I got lost in graph.c :-(.

I think this is the correct answer.  But now I've found that git log
--graph -c -p doesn't indent the diff - that seems to be a separate
issue.

-- 8 --

When showing merges in git-log, the same commit is shown once for each
parent.  Combined with --graph this results in graph_show_commit()
being called once for each parent without graph_update() being called.

Currently graph_show_commit() does not print anything on subsequent
invocations for the same commit (this was changed by commit 656197a -
graph.c: infinite loop in git whatchanged --graph -m from the previous
behaviour of looping infinitely).

Change this so that if the graph code believes it has already shown the
commit it prints a single padding line.

Signed-off-by: John Keeping j...@keeping.me.uk
---
 graph.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/graph.c b/graph.c
index 391a712..2a3fc5c 100644
--- a/graph.c
+++ b/graph.c
@@ -1227,6 +1227,16 @@ void graph_show_commit(struct git_graph *graph)
if (!graph)
return;
 
+   /*
+* When showing a diff of a merge against each of its parents, we
+* are called once for each parent without graph_update having been
+* called.  In this case, simply output a single padding line.
+*/
+   if (graph_is_commit_finished(graph)) {
+   graph_show_padding(graph);
+   shown_commit_line = 1;
+   }
+
while (!shown_commit_line  !graph_is_commit_finished(graph)) {
shown_commit_line = graph_next_line(graph, msgbuf);
fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout);
-- 
1.8.1.2

--
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] test-lib.sh: No POSIXPERM for cygwin

2013-02-06 Thread Torsten Bögershausen

Am 2013-02-06 10:34, schrieb Erik Faye-Lund:

On Sun, Jan 27, 2013 at 3:57 PM, Torsten Bögershausen tbo...@web.de wrote:

t0070 and t1301 fail when running the test suite under cygwin.
Skip the failing tests by unsetting POSIXPERM.



But is this the real reason? I thought Cygwin implemented POSIX permissions...?

t0070:
 'mktemp to unwritable directory prints filename'
  mkdir cannotwrite 
  chmod -w cannotwrite 
  test_when_finished chmod +w cannotwrite 
  test_must_fail test-mktemp cannotwrite/testXX 2err 
  grep cannotwrite/test err

When a directory under Linux/*nix has no write permission,
it is not allowed to create another directory (or file..) here.
This is not working under cygwin, a directory/file can be created
even if the parent directory has chmod 0.
-
tb@PC /cygdrive/c/temp
$ mkdir ttt

tb@PC /cygdrive/c/temp
$ chmod 0 ttt

tb@PC /cygdrive/c/temp
$ ls -ld ttt
d-+ 1 tb None 0 Feb  6 20:33 ttt

tb@PC /cygdrive/c/temp
$ touch ttt/x

tb@PC /cygdrive/c/temp
$ ls -ld ttt
d-+ 1 tb None 0 Feb  6 20:33 ttt

tb@PC /cygdrive/c/temp
$ ls -l ttt
total 0
-rw-r--r--+ 1 tb None 0 Feb  6 20:33 x
---

If this is POSIX compliant? I'm not an expert here.
On the other hand:
This test case does not test git, but rather the file system,
so we can probaly remove it?

About 1301:
Some resereach needs to be done, to find out the connection between
umask, cygwin and the mount options.

On my system I have:
$mount
C: on /cygdrive/c type ntfs (binary,posix=0,user,noumount,auto)

/Torsten




--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/4] Make git-send-email git-credential

2013-02-06 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

As discussed on the list, adding git-credential interface to Git.pm
(sort of copied from git-remote-mediawiki) and making git-send-email
use it.

I see git-remote-mediawiki does not have “use Git” so I did not touch
it.  On top of that I'd have no way to tests the changes anyway.

Michal Nazarewicz (4):
  Git.pm: Allow command_close_bidi_pipe() to be called as method
  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  |  60 +++-
 perl/Git.pm  | 116 ++-
 3 files changed, 149 insertions(+), 31 deletions(-)

-- 
1.8.1.2.550.g0d3a9c0.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 0/4] Make git-send-email git-credential

2013-02-06 Thread Michal Nazarewicz
On Wed, Feb 06 2013, Michal Nazarewicz wrote:
 As discussed on the list, adding git-credential interface to Git.pm
 (sort of copied from git-remote-mediawiki) and making git-send-email
 use it.

 I see git-remote-mediawiki does not have “use Git” so I did not touch
 it.  On top of that I'd have no way to tests the changes anyway.

 Michal Nazarewicz (4):
   Git.pm: Allow command_close_bidi_pipe() to be called as method
   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  |  60 +++-
  perl/Git.pm  | 116 
 ++-
  3 files changed, 149 insertions(+), 31 deletions(-)

On second thought, give me a moment, ;) I've just discovered a bug
preventing git-send-email from mailing a patchset.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +email/xmpp: m...@google.com--ooO--(_)--Ooo--

pgpHpgySqvgfA.pgp
Description: PGP signature


Re: Why is ident_is_sufficient different on Windows?

2013-02-06 Thread Junio C Hamano
Max Horn m...@quendi.de writes:

 static int ident_is_sufficient(int user_ident_explicitly_given)
 {
 #ifndef WINDOWS
   return (user_ident_explicitly_given  IDENT_MAIL_GIVEN);
 #else
   return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
 #endif
 }


 According to git blame, this was introduced here:

 commit 5aeb3a3a838b2cb03d250f3376cf9c41f4d4608e
 Author: Junio C Hamano gits...@pobox.com
 Date:   Sun Jan 17 13:54:28 2010 -0800

 user_ident_sufficiently_given(): refactor the logic to be usable from 
 elsewhere


 The commit message sounds as if this was only a refactoring, but
 the patch to me look as if it changes behaviour, too. Of course
 this could very well be false, say due to code elsewhere that
 already caused Windows to behave differently; I wouldn't know.

 Still, I wonder: Why does this difference exist?

Sorry but I do not recall why these ifdefs are there.  The commit
did this to builtin-commit.c:

-   if (user_ident_explicitly_given != IDENT_ALL_GIVEN)
+   if (!user_ident_sufficiently_given())

I would have written the function to always check with ALL_GIVEN
myself, and it is very likely that I was *not* the person who
noticed that the function needs to behave differently on Windows, as
I do not do Windows.

I suspect somebody from the Windows camp saw a patch I posted
without the ifdef, noticed that there is a problem to expect
IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted
in a reroll of the function in that shape.

I didn't find anything in the list archive, though.  So I am
stumped.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/4] Make git-send-email git-credential

2013-02-06 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@mina86.com

As discussed on the list, adding git-credential interface to Git.pm
(sort of copied from git-remote-mediawiki) and making git-send-email
use it.

I see git-remote-mediawiki does not have “use Git” so I did not touch
it.  On top of that I'd have no way to tests the changes anyway.

Michal Nazarewicz (4):
  Git.pm: Allow command_close_bidi_pipe() to be called as method
  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  | 116 ++-
 3 files changed, 149 insertions(+), 30 deletions(-)

-- 
1.8.1.2.549.g4fa355e

--
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/4] Git.pm: Allow command_close_bidi_pipe() to be called as method

2013-02-06 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@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 min...@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.g4fa355e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] Git.pm: Allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-06 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@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.

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);

Signed-off-by: Michal Nazarewicz min...@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..6a2d52d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -432,7 +432,7 @@ 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.g4fa355e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] git-send-email: Use git credential to obtain password.

2013-02-06 Thread Michal Nazarewicz
From: Michal Nazarewicz min...@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 min...@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.g4fa355e

--
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: Why is ident_is_sufficient different on Windows?

2013-02-06 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 I suspect somebody from the Windows camp saw a patch I posted
 without the ifdef, noticed that there is a problem to expect
 IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted
 in a reroll of the function in that shape.

 I didn't find anything in the list archive, though.  So I am
 stumped.

The only thing I can think of is that on Unix we can guess name from
GECOS, which could be considered sufficiently your name, while on
Windows we probably do not get anything useful there.
--
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: Why is ident_is_sufficient different on Windows?

2013-02-06 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Junio C Hamano gits...@pobox.com writes:

 I suspect somebody from the Windows camp saw a patch I posted
 without the ifdef, noticed that there is a problem to expect
 IDENT_NAME_GIVEN to be set on Windows for some reason, and resulted
 in a reroll of the function in that shape.

 I didn't find anything in the list archive, though.  So I am
 stumped.

 The only thing I can think of is that on Unix we can guess name from
 GECOS, which could be considered sufficiently your name, while on
 Windows we probably do not get anything useful there.

http://thread.gmane.org/gmane.comp.version-control.git/137312/focus=137345

These days, we encourage setting user.name explicitly even on a
system on which it is likely that we will see a good GECOS value, so
removing the ifdef and always check with ALL may not hurt anybody.
I dunno.
--
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/4] Make git-send-email git-credential

2013-02-06 Thread Junio C Hamano
Michal Nazarewicz min...@mina86.com writes:

 On second thought, give me a moment, ;) I've just discovered a bug
 preventing git-send-email from mailing a patchset.

I somehow found this highly amusing.

I wish all the bugs are like that: if your series is buggy, some
parts of the system prevents you from sending it to the list.

;-)
--
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 v4] submodule: add 'deinit' command

2013-02-06 Thread Jens Lehmann
With git submodule init the user is able to tell git he cares about one
or more submodules and wants to have it populated on the next call to git
submodule update. But currently there is no easy way he could tell git he
does not care about a submodule anymore and wants to get rid of his local
work tree (except he knows a lot about submodule internals and removes the
submodule.$name.url setting from .git/config together with the work tree
himself).

Help those users by providing a 'deinit' command. This removes the whole
submodule.name section from .git/config either for the given
submodule(s) or for all those which have been initialized if '.' is
given. Fail if the current work tree contains modifications unless
forced. Complain when for a submodule given on the command line the url
setting can't be found in .git/config, but nonetheless don't fail.

Add tests and link the man pages of git submodule deinit and git rm
to assist the user in deciding whether removing or unregistering the
submodule is the right thing to do for him.

Signed-off-by: Jens Lehmann jens.lehm...@web.de
---

Changes since v3:
- Add deinit to the --force documentation of git submodule
- Never remove submodules containing a .git dir, even when forced
- Diagnostic output when rm -rf or mkdir fails
- More test cases


 Documentation/git-rm.txt|   4 ++
 Documentation/git-submodule.txt |  18 +++-
 git-submodule.sh|  78 ++-
 t/t7400-submodule-basic.sh  | 100 
 4 files changed, 198 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 92bac27..1d876c2 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -149,6 +149,10 @@ files that aren't ignored are present in the submodules 
work tree.
 Ignored files are deemed expendable and won't stop a submodule's work
 tree from being removed.

+If you only want to remove the local checkout of a submodule from your
+work tree without committing the removal,
+use linkgit:git-submodule[1] `deinit` instead.
+
 EXAMPLES
 
 `git rm Documentation/\*.txt`::
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index a0c9df8..bc06159 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -13,6 +13,7 @@ SYNOPSIS
  [--reference repository] [--] repository [path]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
 'git submodule' [--quiet] init [--] [path...]
+'git submodule' [--quiet] deinit [-f|--force] [--] path...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch] [--rebase]
  [--reference repository] [--merge] [--recursive] [--] 
[path...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n]
@@ -134,6 +135,19 @@ init::
the explicit 'init' step if you do not intend to customize
any submodule locations.

+deinit::
+   Unregister the given submodules, i.e. remove the whole
+   `submodule.$name` section from .git/config together with their work
+   tree. Further calls to `git submodule update`, `git submodule foreach`
+   and `git submodule sync` will skip any unregistered submodules until
+   they are initialized again, so use this command if you don't want to
+   have a local checkout of the submodule in your work tree anymore. If
+   you really want to remove a submodule from the repository and commit
+   that use linkgit:git-rm[1] instead.
++
+If `--force` is specified, the submodule's work tree will be removed even if
+it contains local modifications.
+
 update::
Update the registered submodules, i.e. clone missing submodules and
checkout the commit specified in the index of the containing repository.
@@ -213,8 +227,10 @@ OPTIONS

 -f::
 --force::
-   This option is only valid for add and update commands.
+   This option is only valid for add, deinit and update commands.
When running add, allow adding an otherwise ignored submodule path.
+   When running deinit the submodule work trees will be removed even if
+   they contain local changes.
When running update, throw away local changes in submodules when
switching to a different commit; and always run a checkout operation
in the submodule, even if the commit listed in the index of the
diff --git a/git-submodule.sh b/git-submodule.sh
index 004c034..f1b552f 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,6 +8,7 @@ dashless=$(basename $0 | sed -e 's/-/ /')
 USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference 
repository] [--] repository [path]
or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...]
or: $dashless [--quiet] init [--] [path...]
+   or: $dashless [--quiet] deinit [-f|--force] [--] path...
or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 

Re: will git provide `submodule remove` option ?‏

2013-02-06 Thread Jens Lehmann
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?
--
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/8] Hiding refs

2013-02-06 Thread Michael Haggerty
On 02/06/2013 08:17 PM, Junio C Hamano wrote:
 Duy Nguyen pclo...@gmail.com writes:
 
 On Tue, Feb 5, 2013 at 5:29 PM, Michael Haggerty mhag...@alum.mit.edu 
 wrote:
 Hiderefs creates a dark corner of a remote git repo that can hold
 arbitrary content that is impossible for anybody to discover but
 nevertheless possible for anybody to download (if they know the name of
 a hidden reference).  In earlier versions of the patch series I believe
 that it was possible to push to a hidden reference hierarchy, which made
 it possible to upload dark content.  The new version appears (from the
 code) to prohibit adding references in a hidden hierarchy, which would
 close the main loophole that I was worried about.  But the documentation
 and the unit tests only explicitly say that updates and deletes are
 prohibited; nothing is said about adding references (unless update is
 understood to include add).  I think the true behavior should be
 clarified and tested.

 I was worried that somehow this dark content could be used for
 malicious purposes; for example, pushing compromised code then
 convincing somebody to download it by SHA1 with the implicit argument
 it's safe since it comes directly from the project's official
 repository.  If it is indeed impossible to populate the dark namespace
 remotely then I can't think of a way to exploit it.

 Or you can think hiderefs is the first step to addressing the
 initial ref advertisment problem.  The series says hidden refs are
 to be fetched out of band, but that's not the only way.
 
 Let me help unconfuse this thread.
 
 I think the series as 8-patch series was poorly presented, and
 separating it into two will help understanding what they are about.
 
 The first three:
 
   upload-pack: share more code
   upload-pack: simplify request validation
   upload/receive-pack: allow hiding ref hierarchies
 
 is _the_ topic of the series.  As far as I am concerned (I am not
 speaking for Gerrit users, but am speaking as the Git maintainer),
 the topic is solely about uncluttering.  There may be refs that the
 server end may need to keep for its operation, but that remote users
 have _no_ business knowing about.  Allowing the server to keep these
 refs in the repository, while not showing these refs over the wire,
 is the problem the series solves.
 
 In other words, it is not about these are *usually* not wanted by
 clients, so do not show them by default.  It is about these are
 not to be shown, ever.
 
 OK?

Yes, the first three patches sound much more reasonable if this is the
goal.  Do you know of users who want the feature defined by the first
three patches, or is it only a stepping stone towards an actually useful
feature?  (I ask because I have trouble imagining a real-world scenario
where these alone would be useful.)

 Now, there may be some refs that are not *usually* wanted by clients
 but there may be cases where clients want to
 
  (1) learn about them via the same protocol; and/or
  (2) fetch them over the protocol.
 
 If you want to solve both of these two issues generally, the
 solution has to involve a separate protocol from the today's
 protocol.  It would go like this:
[... omitted clear explanation of how delayed advertisement could be
implemented via a new protocol ...]

 But in the meantime, if there is a niche use case where a solution
 to only the second problem is sufficient (and Gerrit and GitHub pull
 requests could both be such use cases), the remainder of the series
 can help, without waiting the solution to solve usually not wanted
 but may need to be learned problem.  That is the latter 4 patches
 (the very last one is a demonstration to illustrate why allowing a
 push to hidden ref hierarchy would not and should not work, and is
 not for application):

Given that some people *do* want to fetch all pull requests, is this a
feature that any hosting service would really turn on?  True, the
majority of users would be spared clutter, but at the cost of completely
preventing other users from fetching all pull requests, mirroring the
repository, etc.

In other words, I wonder whether your two incremental steps are useful
at all, in the real world, without yet-to-be-implemented future changes.
 If not, then it doesn't make sense to merge them without at least
imagining the final goal and gaining confidence that they are not false
starts.


I think that a more useful interim solution would be to make it easy to
have two URLs accessing a single git repository, with different levels
of reference visibility applied to each.  This is something that
providers could turn on without sacrificing any existing functionality.
 And it would solve all three problems: clutter, bandwidth, and provenance.

Your first three patches would allow two-tier access to be implemented,
for example by setting GIT_CONFIG or GIT_CONFIG_PARAMETERS or
command-line parameters differently for the processes serving the two
URLs, like:

git upload-pack ...

vs.


Re: [PATCH] git-send-email: add ~/.authinfo parsing

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 10:58:13AM -0500, Ted Zlatanov wrote:

 MM I don't know about the netrc credential helper, but I guess that's
 MM another layer. The git-remote-mediawiki code is the code to call the
 MM credential C API, that in turn may (or may not) call a credential
 MM helper.
 
 Yup.  But what you call read and write are, to the credential
 helper, write and read but it's the same protocol :)  So maybe the
 names should be changed to reflect that, e.g. query and response.

Is that true? As a user of the credential system, git-remote-mediawiki
would want to write to git-credential, then read the response. As a
helper, git-credential-netrc would want to read the query then
write the response. The order is different, but the operations should
be the same in both cases.

The big difference is that mediawiki would want an additional function
to open a pipe to git credential and operate on that, whereas the
helper will be reading/writing stdio.

-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] connect.c: Tell *PLink to always use ssh protocol

2013-02-06 Thread Sven Strickroth
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.

Signed-off-by: Sven Strickroth em...@cs-ware.de
---
 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) {
-- 
Best regards,
 Sven Strickroth
 PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/4] show: obey --textconv for blobs

2013-02-06 Thread Jeff King
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?

  {
 + 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?

Would it be better if object_array_entry replaced its mode member with
an object_context? The only downside I see is that we might waste a
significant amount of memory (each context has a PATH_MAX buffer in it).

-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/8] Hiding refs

2013-02-06 Thread Michael Haggerty
On 02/06/2013 08:55 PM, Jonathan Nieder wrote:
 Michael Haggerty wrote:
 [...]
 But now every time I do a gitk --all or git log --decorate, the
 output is cluttered with all of his references (most of which are just
 old versions of references from the upstream repository that we both
 use).  I would like to be able to hide his references most of the time
 but turn them back on when I need them.

 Scenario 5: Our upstream repository has gazillions of release tags under
 refs/tags/releases/..., sometimes including customer-specific
 releases.  In my daily life these are just clutter.
 
 For both of these use cases, putting the refs somewhere other than
 refs/heads, refs/tags, and refs/remotes should be enough to avoid
 clutter.

Thanks, yes, for release tags in particular your suggestion might be
workable.  But I also like the idea of being able to turn subsets of
references on and off easily, and have the choice persist until I change it.

 [...]
 * Some small improvements (e.g. allowing *multiple* views to be
   defined) would provide much more benefit for about the same effort,
   and would be a better base for building other features in the future
   (e.g., local views).
 
 Would advertising GIT_CONFIG_PARAMETERS and giving examples for server
 admins to set it in inetd et al to provide different kinds of access
 to a same repository through different URLs work?
 
 Thanks for listening.
 Michael

 [1] Theoretically one could support multiple views of a single
 repository by using something like GIT_CONFIG=view_1_config git
 upload-pack ... or git -c transfer.hiderefs=... git upload-pack ...,
 but this would be awkward.
 
 Ah, I missed this comment before.  What's awkward about that?  I
 think it's a clean way to make many aspects of how a repository is
 presented (including hook actions) configurable.

Awkwardness using GIT_CONFIG: the admin would have to maintain two
separate config files with mostly overlapping content.

Awkwardness using GIT_CONFIG_PARAMETERS or -c transfer.hiderefs=...:
the hiderefs configuration would have to be maintained in some Apache
config or inetd or ... (or multiple places!) rather than in the
repository's config file, where it belongs.

Additional awkwardness using -c transfer.hiderefs=...: AFAIK there is
no way to turn *off* a configuration variable via a command-line option.

It's all doable, but I find it awkward.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/4] show: obey --textconv for blobs

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 08:53:52AM -0800, Junio C Hamano wrote:

 Michael J Gruber g...@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?
 
 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.
 
 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 think Michael's commit message explains the situation badly.
--textconv is already on for git show (and for git log) by default.
Diffs already use it.

This is more about the fact that when showing a single blob, we do not
bother to remember the context of the sha1 lookup, including its
pathname. Therefore we were not previously able to apply any
.gitattributes to the output. So this patch really does two things:

  1. Pass the information along to show_blob_object so that it can
 look up .gitattributes.

  2. Apply the textconv attribute (if ALLOW_TEXTCONV is on, of course).

And stating it that way makes it clear that there may be other missing
steps (3 and up) to apply other gitattributes. For example, should git
show $blob respect crlf attributes? Smudge filters?

-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/8] Hiding refs

2013-02-06 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 On 02/06/2013 08:17 PM, Junio C Hamano wrote:
 ...

 Yes, the first three patches sound much more reasonable if this is the
 goal.
 ...
 I think that a more useful interim solution would be to make it easy to
 have two URLs accessing a single git repository, with different levels
 of reference visibility applied to each.

I think you said more reasonable without understanding what you
are saying is more reasonable, then.  The mechanism is about
server side wanting to use refs to protect its own metadata from gc
without having to expose them; there is no different levels.

 ...
 GIT_CONFIG=config-with-hidden-refs git upload-pack ...
 or
 git -c transfer.hiderefs=refs/pull upload-pack ...

 But this is a bit awkward ...

It is awkward to use hammer to drive screws in wood, too.  You want
to use a screwdriver.  The first three patches are to drive a nail
with hammer, OK?  Screws you keep bringing up is to be handled by
delayed ref advertisement.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 2/4] cat-file: do not die on --textconv without textconv filters

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 04:08:51PM +0100, Michael J Gruber wrote:

 When a command is supposed to use textconv filters (by default or with
 --textconv) and none are configured then the blob is output without
 conversion; the only exception to this rule is cat-file --textconv.
 
 Make it behave like the rest of textconv aware commands.

Makes sense.

 - if (!textconv_object(obj_context.path, obj_context.mode, sha1, 
 1, buf, size))
 - die(git cat-file --textconv: unable to run textconv on 
 %s,
 - obj_name);
 - break;
 + if (textconv_object(obj_context.path, obj_context.mode, sha1, 
 1, buf, size))
 + break;

The implication here is that textconv_object should be handling its own
errors and dying, and the return is always yes, I converted or no, I
did not. Which I think is the case.

 +
 + /* otherwise expect a blob */
 + exp_type = blob;
  
   case 0:
   if (type_from_string(exp_type) == OBJ_BLOB) {

I wondered at first why we needed to set exp_type here; shouldn't we
already be expecting a blob if we are doing textconv? But then I see
this is really about the fall-through in the switch (which we might want
an explicit comment for).

Which made me wonder: what happens with:

  git cat-file --textconv HEAD

It looks like we die just before textconv-ing, because we have no
obj_context.path. But that is also unlike all of the other --textconv
switches, which mean turn on textconv if you are showing a blob that
supports it and not the specific operation is --textconv, apply it to
this blob. I don't know if that is worth changing 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: [RFC/PATCH 3/4] grep: allow to use textconv filters

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 04:08:52PM +0100, Michael J Gruber wrote:

 From: Jeff King p...@peff.net
 
 Recently and not so recently, we made sure that log/grep type operations
 use textconv filters when a userfacing diff would do the same:
 
 ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
 b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
 
 git grep currently does not use textconv filters at all, that is
 neither for displaying the match and context nor for the actual grepping.
 
 Introduce an option --textconv which makes git grep use any configured
 textconv filters for grepping and output purposes. It is off by default.
 
 Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net

Signed-off-by: Jeff King p...@peff.net

I'd really love to see the refactoring I talked about in my earlier
message. But as I'm not willing to devote the time to do it right now,
and I do not think this patch has any particular bugs, I think it is OK
as it gets the job done, and does not make the later refactoring any
harder.

The one ugliness that still remains is:

 + if (opt-allow_textconv) {
 + grep_source_load_driver(gs);
 + /*
 +  * We might set up the shared textconv cache data here, which
 +  * is not thread-safe.
 +  */
 + grep_attr_lock();
 + textconv = userdiff_get_textconv(gs-driver);
 + grep_attr_unlock();
 + }

We lock/unlock the grep_attr_lock twice here: once in
grep_source_load_driver, and then immediately again to call
userdiff_get_textconv. I don't know if it is worth doing the two under
the same lock or not (I guess it should not increase lock contention,
since we do the same amount of work, so it is really just the extra lock
instructions).

-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/PATCH 2/4] cat-file: do not die on --textconv without textconv filters

2013-02-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Which made me wonder: what happens with:

   git cat-file --textconv HEAD

 It looks like we die just before textconv-ing, because we have no
 obj_context.path. But that is also unlike all of the other --textconv
 switches, which mean turn on textconv if you are showing a blob that
 supports it and not the specific operation is --textconv, apply it to
 this blob. I don't know if that is worth changing or not.

OK, so in that sense, cat-file --textconv HEAD (or HEAD:) should
die with Hey, that is not a blob, in other words, Michael's patch
does what we want without further tweaks, right?

By the way are we sure textconv_object() barfs and dies if fed a non
blob?  Otherwise the above does not hold, and the semantics become
turn on textconv if the object you are showing supports it,
otherwise it has to be a blob., I think.
--
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/8] Hiding refs

2013-02-06 Thread Ævar Arnfjörð Bjarmason
On Wed, Feb 6, 2013 at 8:17 PM, Junio C Hamano gits...@pobox.com wrote:

Maybe this should be split up into a different thread, but:

 The upload-pack-2 service sits on a port different from today's
 [...].

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.

We already covered in previous discussions how this would be simpler
with the HTTP protocol, since you could just send an extra header
inviting the server to speak the new protocol.

But for the other transports we can just try the new protocol and
retry with the old one as a fallback if it doesn't work. That'll allow
us to gracefully migrate without needing to change the git:// port.

Besides, I think the vast majority of users are using Git via http://
or ssh://, where we can't just change the port, but even so making
people change the port when we could handle this more gracefully would
be a big PITA. Adding new firewall holes is often a big bureaucratic
nightmare in some organizations.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 4/4] grep: obey --textconv for the case rev:path

2013-02-06 Thread Jeff King
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
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.

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.

-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/PATCH 2/4] cat-file: do not die on --textconv without textconv filters

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 02:23:36PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  Which made me wonder: what happens with:
 
git cat-file --textconv HEAD
 
  It looks like we die just before textconv-ing, because we have no
  obj_context.path. But that is also unlike all of the other --textconv
  switches, which mean turn on textconv if you are showing a blob that
  supports it and not the specific operation is --textconv, apply it to
  this blob. I don't know if that is worth changing or not.
 
 OK, so in that sense, cat-file --textconv HEAD (or HEAD:) should
 die with Hey, that is not a blob, in other words, Michael's patch
 does what we want without further tweaks, right?

Right, it will die because we do not find a path in the object_context.
For the same reason that cat-file --textconv $sha1 would die.

A more interesting case is cat-file --textconv HEAD:Documentation,
which does have a path, but not a blob. And I think that speaks to your
point that we want to fall-through to the pretty-print case, not the
blob case.

 By the way are we sure textconv_object() barfs and dies if fed a non
 blob?  Otherwise the above does not hold, and the semantics become
 turn on textconv if the object you are showing supports it,
 otherwise it has to be a blob., I think.

I'm not sure. The sha1 would get passed all the way down to
fill_textconv. I think because sha1_valid is set, it will not try to
reuse the working tree file, so we will end up in
diff_populate_filespec, and we could actually textconv the tree object
itself.

So yeah, I think we want a check that makes sure we are working with a
blob before we even call that function, and --textconv should just be
you must feed me a blob of the form $treeish:$path. In practice nobody
wants to do anything else anyway, so let's keep the code paths simple;
we can always loosen it later if there is a good reason to do so.

-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] Verify Content-Type from smart HTTP servers

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 07:56:08AM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  Is it worth having a strbuf_set* family of functions to match the
  strbuf_add*? We seem to have these sorts of errors with strbuf from time
  to time, and I wonder if that would make it easier (and more readable)
  to do the right thing.
 
 Possibly.
 
 The callsite below may be a poor example, though; you would need the
 _reset() even if you change the _addstr() we can see in the context
 to _setstr() to make sure later strbuf_*(type) will start from a
 clean slate when !t anyway, no?

Ah, true. Let's not worry about it, then.

-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/8] Hiding refs

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 11:17:06AM -0800, Junio C Hamano wrote:

 Let me help unconfuse this thread.
 
 I think the series as 8-patch series was poorly presented, and
 separating it into two will help understanding what they are about.
 
 The first three:
 
   upload-pack: share more code
   upload-pack: simplify request validation
   upload/receive-pack: allow hiding ref hierarchies
 
 is _the_ topic of the series.  As far as I am concerned (I am not
 speaking for Gerrit users, but am speaking as the Git maintainer),
 the topic is solely about uncluttering.  There may be refs that the
 server end may need to keep for its operation, but that remote users
 have _no_ business knowing about.  Allowing the server to keep these
 refs in the repository, while not showing these refs over the wire,
 is the problem the series solves.
 
 In other words, it is not about these are *usually* not wanted by
 clients, so do not show them by default.  It is about these are
 not to be shown, ever.
 
 OK?

Right. I am not opposed to this series, as it does have a use-case. And
if it helps Gerrit folks or other users unclutter, great. The fact that
I could throw away the custom receive.hiderefs patch we use at GitHub is
a bonus. If people want fancier things, they can do them separately.

_But_. As a potential user of the feature (to hide refs/pull/*), I do
not think it is sufficiently flexible for me to use transfer.hiderefs
(or uploadpack.hiderefs). We use fetch internally to migrate objects
between forks and our alternates repos. And in that case, we really do
want to see all refs. In other words, all fetches are not the same: we
would want upload-pack to understand the difference between a client
fetch and an internal administrative fetch. But this feature does not
provide that lee-way. Even if you tried:

  git fetch -u 'git -c uploadpack.hiderefs= upload-pack'

the list nature of the config variable means you cannot reset it.

This isn't a show-stopper for the series; it may just mean that it is
not a good fit for GitHub's use case, but others (like Gerrit) may
benefit. But since refs/pull is used as an example of where this could
be applied, I wanted to point out that it does not achieve that goal.

-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/4] Git.pm: Allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 09:47:04PM +0100, Michal Nazarewicz wrote:

 From: Michal Nazarewicz min...@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.
 
 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);

Should this part go into the documentation for command_close_bidi_pipe
in Git.pm?

-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-send-email: add ~/.authinfo parsing

2013-02-06 Thread Ted Zlatanov
On Wed, 6 Feb 2013 16:57:24 -0500 Jeff King p...@peff.net wrote: 

JK On Wed, Feb 06, 2013 at 10:58:13AM -0500, Ted Zlatanov wrote:
MM I don't know about the netrc credential helper, but I guess that's
MM another layer. The git-remote-mediawiki code is the code to call the
MM credential C API, that in turn may (or may not) call a credential
MM helper.
 
 Yup.  But what you call read and write are, to the credential
 helper, write and read but it's the same protocol :)  So maybe the
 names should be changed to reflect that, e.g. query and response.

JK Is that true? As a user of the credential system, git-remote-mediawiki
JK would want to write to git-credential, then read the response. As a
JK helper, git-credential-netrc would want to read the query then
JK write the response. The order is different, but the operations should
JK be the same in both cases.

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.

JK The big difference is that mediawiki would want an additional function
JK to open a pipe to git credential and operate on that, whereas the
JK helper will be reading/writing stdio.

Yup.

Ted
--
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/4] Git.pm: Add interface for git credential command.

2013-02-06 Thread Jeff King
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.

 +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?

 +In the second form, CCODE needs to be a reference to a subroutine.
 +The function will execute Cgit credential fill to fill provided
 +credential hash, than call CCODE with CCREDENTIAL as the sole
 +argument, and finally depending on CCODE's return value execute
 +Cgit credential approve (if return value yields true) or Cgit
 +credential reject (otherwise).  The return value is the same as what
 +CCODE returned.  With this form, the usage might look as follows:

This is a nice touch. It makes the normal calling code a lot simpler.

-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 4/4] git-send-email: Use git credential to obtain password.

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 09:47:06PM +0100, Michal Nazarewicz wrote:

 From: Michal Nazarewicz min...@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 min...@mina86.com
 ---
  Documentation/git-send-email.txt |  4 +--
  git-send-email.perl  | 59 
 +++-
  2 files changed, 36 insertions(+), 27 deletions(-)

Nice. I don't see anything obviously wrong with the code, but I didn't
try it myself. I wonder how hard it would be to have some tests in
t9001. It looks like we don't test the smtp code paths at all, since we
would have to implement a fake smtp server. Which probably means the
answer is is pretty hard, unless there is an easy-to-use CPAN smtp
server module we can plug in.

-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] connect.c: Tell *PLink to always use ssh protocol

2013-02-06 Thread 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.
 
 Signed-off-by: Sven Strickroth em...@cs-ware.de

Makes sense to me, though I'd expect to see this cc'd to the msysgit
list (which I'm doing on this response) for comment from people who
might be more familiar with the area.

Quoted patch follows.

-Peff

 ---
  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) {
 -- 
 Best regards,
  Sven Strickroth
  PGP key id F5A9D4C4 @ any key-server
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/4] show: obey --textconv for blobs

2013-02-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 And stating it that way makes it clear that there may be other missing
 steps (3 and up) to apply other gitattributes. For example, should git
 show $blob respect crlf attributes? Smudge filters?

Yeah, I think applying these when path is availble may make sense.

Are we going to teach cat-file to honor them with --textconv and
possibly other new options?

Or should the --textconv imply application of these other filters?
I am tempted to say yes, but I haven't thought things through...

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 1/4] show: obey --textconv for blobs

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 03:49:44PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  And stating it that way makes it clear that there may be other missing
  steps (3 and up) to apply other gitattributes. For example, should git
  show $blob respect crlf attributes? Smudge filters?
 
 Yeah, I think applying these when path is availble may make sense.
 
 Are we going to teach cat-file to honor them with --textconv and
 possibly other new options?
 
 Or should the --textconv imply application of these other filters?
 I am tempted to say yes, but I haven't thought things through...

For diff, we should already recognize them (because we feed the diff
machinery the path name, and it does the usual attributes lookup). Of
course it only uses things like funcname, not any of the
convert-to-filesystem options (hmm, actually, I guess it may use
convert-from-filesystem, but in such a case it would not be reading from
a blob anyway, but from a filesystem path, so it is not related to this
code).

For cat-file, I don't think --textconv should necessarily imply it. The
original motivation for cat-file --textconv was for external blame to
be able to access the converted contents. But it would not want to do
filesystem-level conversion; it wants the canonical version of the file.
I think a better option name for smudge/crlf would be --to-filesystem
or something like that. And probably it should not be the default.

git-show should probably have the same option. I doubt it should be on
by default, but I can see it being useful for:

  git show --to-filesystem HEAD:Makefile Makefile

or whatever. I don't think those features necessarily need to come as
part of Michael's series. They can wait for people who care to implement
them. But I do think the refactoring of passing along the path from the
object_context should keep in mind that we will probably want to go that
way eventually.

Are there other attributes that we might care about when showing a blob?
The only ones I can think of are the ones for converting to the
filesystem representation.

-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/8] Hiding refs

2013-02-06 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason ava...@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)?

 We already covered in previous discussions how this would be simpler
 with the HTTP protocol,...

Yes, that is a solved problem.
--
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/4] Git.pm: Allow pipes to be closed prior to calling command_close_bidi_pipe

2013-02-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Wed, Feb 06, 2013 at 09:47:04PM +0100, Michal Nazarewicz wrote:

 From: Michal Nazarewicz min...@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.
 
 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);

 Should this part go into the documentation for command_close_bidi_pipe
 in Git.pm?

Yeah, it probably should.

--
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/8] Hiding refs

2013-02-06 Thread Jeff King
On Wed, Feb 06, 2013 at 04:12:10PM -0800, Junio C Hamano wrote:

 Ævar Arnfjörð Bjarmason ava...@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).

-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/PATCH 1/4] show: obey --textconv for blobs

2013-02-06 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 git-show should probably have the same option. I doubt it should be on
 by default, but I can see it being useful for:

   git show --to-filesystem HEAD:Makefile Makefile

 or whatever. I don't think those features necessarily need to come as
 part of Michael's series.

Yeah, all makes sense (including the part that it might be useful
but it does not belong to this series).

Thanks for sanity checking.
--
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/3] combine-diff: suppress a clang warning

2013-02-06 Thread Miles Bader
John Keeping j...@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.

-miles 

-- 
永日の 澄んだ紺から 永遠へ
--
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: will git provide `submodule remove` option ?‏

2013-02-06 Thread Lingcha X
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.lehm...@web.de
 To: dougla...@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?   
   

Re: [PATCH] git-send-email: add ~/.authinfo parsing

2013-02-06 Thread Matthieu Moy
Ted Zlatanov t...@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/
--
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/4] Git.pm: Add interface for git credential command.

2013-02-06 Thread Matthieu Moy
Michal Nazarewicz m...@google.com writes:

 From: Michal Nazarewicz min...@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/
--
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/4] Git.pm: Add interface for git credential command.

2013-02-06 Thread Matthieu Moy
Michal Nazarewicz m...@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/
--
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


  1   2   >