Re: importing two different trees into a fresh git repo
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
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
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
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
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
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
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
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
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?
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
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
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
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)
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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)
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)
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
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
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
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
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)
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
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)
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
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
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
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
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
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
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
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?
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
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
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
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.
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?
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?
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
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
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 ?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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
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
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
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
Æ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
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
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
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
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 ?
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
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.
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.
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