* Re: [PATCH] push: Alias pushurl from push rewrites
From: Rob Hoelz @ 2013-03-18 10:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, josh
In-Reply-To: <7vd2uxppk0.fsf@alter.siamese.dyndns.org>
On 3/18/13 12:35 AM, Junio C Hamano wrote:
> Rob Hoelz <rob@hoelz.ro> writes:
>
>> git push currently doesn't consider pushInsteadOf when
>> using pushurl; this tests and fixes that.
>>
>> If you use pushurl with an alias that has a pushInsteadOf configuration
>> value, Git does not take advantage of it. For example:
>>
>> [url "git://github.com/"]
>> insteadOf = github:
>> [url "git://github.com/myuser/"]
>> insteadOf = mygithub:
>> [url "git@github.com:myuser/"]
>> pushInsteadOf = mygithub:
>> [remote "origin"]
>> url = github:organization/project
>> pushurl = mygithub:project
> Incomplete sentence? For example [this is an example configuration]
> and then what happens? Something like "with the sample
> configuration, 'git push origin' should follow pushurl and then turn
> it into X, but instead it ends up accessing Y".
>
> If there is no pushInsteadOf, does it still follow insteadOf? Is it
> tested already?
>
> Wouldn't you want to cover all the combinations to negative cases
> (i.e. making sure the codepath to support a new case does not affect
> behaviour of the code outside the new case)? A remote with and
> without pushurl (two combinations) and a pseudo URL scheme with and
> without pushInsteadOf (again, two combinations) will give you four
> cases.
>
>
> Thanks.
Sorry; that's a good point. With the above configuration, the following
fails:
$ git push origin master
With the following message:
fatal: remote error:
You can't push to git://github.com/myuser/project.git
Use git@github.com:myuser/project.git
So you can see that pushurl is being followed (it's not attempting to
push to git://github.com/organization/project), but insteadOf values are
being used as opposed to pushInsteadOf values for expanding the pushurl
alias.
I haven't tried without pushInsteadOf; I will add a test for it later
today. I assume that if pushInsteadOf isn't found, insteadOf should be
used? I will also consider the other test cases you described.
Thanks again for the feedback!
>
>> Signed-off-by: Rob Hoelz <rob@hoelz.ro>
>> ---
>> remote.c | 2 +-
>> t/t5516-fetch-push.sh | 20 ++++++++++++++++++++
>> 2 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/remote.c b/remote.c
>> index ca1f8f2..de7a915 100644
>> --- a/remote.c
>> +++ b/remote.c
>> @@ -465,7 +465,7 @@ static void alias_all_urls(void)
>> if (!remotes[i])
>> continue;
>> for (j = 0; j < remotes[i]->pushurl_nr; j++) {
>> - remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
>> + remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);
>> }
>> add_pushurl_aliases = remotes[i]->pushurl_nr == 0;
>> for (j = 0; j < remotes[i]->url_nr; j++) {
>> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
>> index b5417cc..272e225 100755
>> --- a/t/t5516-fetch-push.sh
>> +++ b/t/t5516-fetch-push.sh
>> @@ -244,6 +244,26 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
>> )
>> '
>>
>> +test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf does rewrite in this case)' '
>> + mk_empty &&
>> + TRASH="$(pwd)/" &&
>> + mkdir ro &&
>> + mkdir rw &&
>> + git init --bare rw/testrepo &&
>> + git config "url.file://$TRASH/ro/.insteadOf" ro: &&
>> + git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&
>> + git config remote.r.url ro:wrong &&
>> + git config remote.r.pushurl rw:testrepo &&
>> + git push r refs/heads/master:refs/remotes/origin/master &&
>> + (
>> + cd rw/testrepo &&
>> + r=$(git show-ref -s --verify refs/remotes/origin/master) &&
>> + test "z$r" = "z$the_commit" &&
>> +
>> + test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
>> + )
>> +'
>> +
>> test_expect_success 'push with matching heads' '
>>
>> mk_test heads/master &&
^ permalink raw reply
* Re: [RFC/PATCH] Documentation/technical/api-fswatch.txt: start with outline
From: Thomas Rast @ 2013-03-18 10:07 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Junio C Hamano, Karsten Blees, Duy Nguyen, Git List,
Torsten Bögershausen, Robert Zeh, Jeff King, Erik Faye-Lund,
Drew Northup
In-Reply-To: <CALkWK0=0+HYn=bSrGC5sMQOE-53As0h9dG3N9wvUB2=NW3=98A@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Junio C Hamano wrote:
>> Yes, and you would need one inotify per directory but you do not
>> have an infinite supply of outstanding inotify watch (wasn't the
>> limit like 8k per a single uid or something?), so the daemon must be
>> prepared to say "I'll watch this, that and that directories, but the
>> consumers should check other directories themselves."
>>
>> FWIW, I share your suspicion that an effort in the direction this
>> thread suggests may end up duplicating what the caching vfs layer
>> already does, and doing so poorly.
>
> Thomas Rast wrote:
>> $ cat /proc/sys/fs/inotify/max_user_watches
>> 65536
>> $ cat /proc/sys/fs/inotify/max_user_instancest
>> 128
>
> From Junio's and Thomas' observations, I'm inclined to think that
> inotify is ill-suited for the problem we are trying to solve. It is
> designed as a per-directory watch, because VFS can quickly supply the
> inodes for a directory entry. As such, I think the ideal usecase for
> inotify is to execute something immediately when a change takes place
> in a directory: it's well-suited for solutions like Dropbox (which I
> think is poorly designed to begin with, but that's offtopic). It
> doesn't substitute of augment VFS caching. I suspect the VFS cache
> works by caching the inodes in a frequently used directory entry, thus
> optimizing calls like lstat() on them.
I have three objections to changing the kernel to fit us, as opposed to
just using inotify:
* inotify works. I can watch most of my $HOME with the hack I linked
earlier[1]. Yes, it's a lot of coding around the problem that it is
nonrecursive, but we already have a lot of code around the problem
that we can't ask the VFS for diffs between points in time (namely,
the whole business with an index and lstat() loops).
* inotify is here today. Even if you got a hypothetical notifier into
the kernel today, you'd have to wait months/years until it is
available in distros, and years until everyone has it.
* I'll bet you a beer that the kernel folks already had the same
discussion when they made inotify. There has to be a reason why it's
better than providing for recursive watches.
[1] https://github.com/trast/watch
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH/RFC] http_init: only initialize SSL for https
From: Erik Faye-Lund @ 2013-03-18 10:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: Daniel Stenberg, Antoine Pelisse, Jeff King, Johannes Schindelin,
git, msysgit
In-Reply-To: <7vli9lpsqe.fsf@alter.siamese.dyndns.org>
On Sun, Mar 17, 2013 at 11:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Daniel Stenberg <daniel@haxx.se> writes:
>
>> On Sun, 17 Mar 2013, Antoine Pelisse wrote:
>>
>>>> With redirects taken into account, I can't think of any really good way
>>>> around avoiding this init...
>>>
>>> Is there any way for curl to initialize SSL on-demand ?
>>
>> Yes, but not without drawbacks.
>>
>> If you don't call curl_global_init() at all, libcurl will notice that
>> on first use and then libcurl will call global_init by itself with a
>> default bitmask.
>>
>> That automatic call of course will prevent the application from being
>> able to set its own bitmask choice, and also the global_init function
>> is not (necessarily) thread safe while all other libcurl functions are
>> so the internal call to global_init from an otherwise thread-safe
>> function is unfortunate.
>
> So in short, unless you are writing a custom application to talk to
> servers that you know will never redirect you to HTTPS, passing
> custom masks such as ALL&~SSL to global-init is not going to be a
> valid optimization.
>
> I think that is a reasonable API; your custom application may want
> to go around your intranet servers all of which serve their status
> over plain HTTP, and it is a valid optimization to initialize the
> library with ALL&~SSL. It is just that such an optimization does
> not apply to us---we let our users go to random hosts we have no
> control over, and they may redirect us in ways we cannot anticipate.
>
I wonder. Our libcurl is build with "-winssl" (USE_WINDOWS_SSPI=1), it
seems. Perhaps switching to openssl (which we already have libraries
for) would make the init-time better?
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* [PATCH v3 4/4] pack-refs: add fully-peeled trait
From: Jeff King @ 2013-03-18 11:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Michael Haggerty
In-Reply-To: <7vli9lre1j.fsf@alter.siamese.dyndns.org>
On Sun, Mar 17, 2013 at 01:01:44PM -0700, Junio C Hamano wrote:
> > + * All references in the file that can be peeled are peeled.
> > + * Inversely (and this is more important, any references in the
>
> A missing closing paren after "more important". Also the e-mail
> quote reveals there is some inconsistent indentation (HTs vs runs of
> SPs) here.
Thanks, will fix (the whitespace damage is due to me cutting and pasting
from Michael's commit).
>
> > + * file for which no peeled value is recorded is not peelable. This
> > + * trait should typically be written alongside "fully-peeled" for
>
> Alongside "peeled", no?
Urgh, yes. Will fix.
> [...]
> I am not sure why you find this any more readable.
I was trying to avoid the "set PEELED globally, but sometimes unset it
for specific refs" pattern which I think is confusing to the reader. But
I think what you wrote is even better. I used an enum rather than two
variables to make it clear that only ones takes effect. I had wanted to
use a switch, also, but you end up either repeating yourself, or doing
this gross fall-through:
switch (peeled) {
case PEELED_TAGS:
if (prefixcmp(refname, "refs/tags/"))
break;
/* fall-through */
case PEELED_FULLY:
last->ref |= REF_KNOWS_PEELED;
break;
default:
/* we know nothing */
}
So I just stuck with the conditional.
Here's the re-roll.
-- >8 --
From: Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH] pack-refs: add fully-peeled trait
Older versions of pack-refs did not write peel lines for
refs outside of refs/tags. This meant that on reading the
pack-refs file, we might set the REF_KNOWS_PEELED flag for
such a ref, even though we do not know anything about its
peeled value.
The previous commit updated the writer to always peel, no
matter what the ref is. That means that packed-refs files
written by newer versions of git are fine to be read by both
old and new versions of git. However, we still have the
problem of reading packed-refs files written by older
versions of git, or by other implementations which have not
yet learned the same trick.
The simplest fix would be to always unset the
REF_KNOWS_PEELED flag for refs outside of refs/tags that do
not have a peel line (if it has a peel line, we know it is
valid, but we cannot assume a missing peel line means
anything). But that loses an important optimization, as
upload-pack should not need to load the object pointed to by
refs/heads/foo to determine that it is not a tag.
Instead, we add a "fully-peeled" trait to the packed-refs
file. If it is set, we know that we can trust a missing peel
line to mean that a ref cannot be peeled. Otherwise, we fall
back to assuming nothing.
[commit message and tests by Jeff King <peff@peff.net>]
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>
---
pack-refs.c | 2 +-
refs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
t/t3211-peel-ref.sh | 22 ++++++++++++++++++++++
3 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/pack-refs.c b/pack-refs.c
index ebde785..4461f71 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -128,7 +128,7 @@ int pack_refs(unsigned int flags)
die_errno("unable to create ref-pack file structure");
/* perhaps other traits later as well */
- fprintf(cbdata.refs_file, "# pack-refs with: peeled \n");
+ fprintf(cbdata.refs_file, "# pack-refs with: peeled fully-peeled \n");
for_each_ref(handle_one_ref, &cbdata);
if (ferror(cbdata.refs_file))
diff --git a/refs.c b/refs.c
index 175b9fc..e2b760d 100644
--- a/refs.c
+++ b/refs.c
@@ -803,11 +803,38 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
return line;
}
+/*
+ * Read f, which is a packed-refs file, into dir.
+ *
+ * A comment line of the form "# pack-refs with: " may contain zero or
+ * more traits. We interpret the traits as follows:
+ *
+ * No traits:
+ *
+ * Probably no references are peeled. But if the file contains a
+ * peeled value for a reference, we will use it.
+ *
+ * peeled:
+ *
+ * References under "refs/tags/", if they *can* be peeled, *are*
+ * peeled in this file. References outside of "refs/tags/" are
+ * probably not peeled even if they could have been, but if we find
+ * a peeled value for such a reference we will use it.
+ *
+ * fully-peeled:
+ *
+ * All references in the file that can be peeled are peeled.
+ * Inversely (and this is more important), any references in the
+ * file for which no peeled value is recorded is not peelable. This
+ * trait should typically be written alongside "peeled" for
+ * compatibility with older clients, but we do not require it
+ * (i.e., "peeled" is a no-op if "fully-peeled" is set).
+ */
static void read_packed_refs(FILE *f, struct ref_dir *dir)
{
struct ref_entry *last = NULL;
char refline[PATH_MAX];
- int flag = REF_ISPACKED;
+ enum { PEELED_NONE, PEELED_TAGS, PEELED_FULLY } peeled = PEELED_NONE;
while (fgets(refline, sizeof(refline), f)) {
unsigned char sha1[20];
@@ -816,15 +843,20 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
if (!strncmp(refline, header, sizeof(header)-1)) {
const char *traits = refline + sizeof(header) - 1;
- if (strstr(traits, " peeled "))
- flag |= REF_KNOWS_PEELED;
+ if (strstr(traits, " fully-peeled "))
+ peeled = PEELED_FULLY;
+ else if (strstr(traits, " peeled "))
+ peeled = PEELED_TAGS;
/* perhaps other traits later as well */
continue;
}
refname = parse_ref_line(refline, sha1);
if (refname) {
- last = create_ref_entry(refname, sha1, flag, 1);
+ last = create_ref_entry(refname, sha1, REF_ISPACKED, 1);
+ if (peeled == PEELED_FULLY ||
+ (peeled == PEELED_TAGS && !prefixcmp(refname, "refs/tags/")))
+ last->flag |= REF_KNOWS_PEELED;
add_ref(dir, last);
continue;
}
@@ -832,8 +864,15 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
refline[0] == '^' &&
strlen(refline) == 42 &&
refline[41] == '\n' &&
- !get_sha1_hex(refline + 1, sha1))
+ !get_sha1_hex(refline + 1, sha1)) {
hashcpy(last->u.value.peeled, sha1);
+ /*
+ * Regardless of what the file header said,
+ * we definitely know the value of *this*
+ * reference:
+ */
+ last->flag |= REF_KNOWS_PEELED;
+ }
}
}
diff --git a/t/t3211-peel-ref.sh b/t/t3211-peel-ref.sh
index 85f09be..d4d7792 100755
--- a/t/t3211-peel-ref.sh
+++ b/t/t3211-peel-ref.sh
@@ -39,4 +39,26 @@ test_expect_success 'refs are peeled outside of refs/tags (packed)' '
test_cmp expect actual
'
+test_expect_success 'create old-style pack-refs without fully-peeled' '
+ # Git no longer writes without fully-peeled, so we just write our own
+ # from scratch; we could also munge the existing file to remove the
+ # fully-peeled bits, but that seems even more prone to failure,
+ # especially if the format ever changes again. At least this way we
+ # know we are emulating exactly what an older git would have written.
+ {
+ echo "# pack-refs with: peeled " &&
+ print_ref "refs/heads/master" &&
+ print_ref "refs/outside/foo" &&
+ print_ref "refs/tags/base" &&
+ print_ref "refs/tags/foo" &&
+ echo "^$(git rev-parse "refs/tags/foo^{}")"
+ } >tmp &&
+ mv tmp .git/packed-refs
+'
+
+test_expect_success 'refs are peeled outside of refs/tags (old packed)' '
+ git show-ref -d >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.2.rc2.7.gef06216
^ permalink raw reply related
* Re: Proposal: sharing .git/config
From: Jeff King @ 2013-03-18 11:48 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Duy Nguyen, Git List
In-Reply-To: <CALkWK0nWXCO_EXfx69m8XbrFe=ABBodPdFbSrS9v3VqgfbnQ+w@mail.gmail.com>
On Mon, Mar 18, 2013 at 02:30:23PM +0530, Ramkumar Ramachandra wrote:
> Jeff King wrote:
> > I don't think you can avoid the 3-step problem and retain the safety in
> > the general case. Forgetting implementation details for a minute, you
> > have either a 1-step system:
> >
> > 1. Fetch and start using config from the remote.
> >
> > which is subject to fetching and executing malicious config, or:
> >
> > 1. Fetch config from remote.
> > 2. Inspect it.
> > 3. Integrate it into the current config.
>
> I don't understand your emphasis on step 2. Isn't the configuration
> written by me? Why would it be malicious?
Maybe I am misunderstanding the use case, but when people talk about
share config, they are often talking about pushing project-wide config
out to developers. So the config is not necessarily written by you, but
by somebody who had write access to the upstream repository.
The obvious counterpoint is that people usually run "make" right after
fetching, so they are trusting what they fetched already. And the
counter-counterpoint is that yes, that's true, but at least with the
"make" case they can use git to inspect the differences before running
them. You may be able to tell that this is not the first time this
discussion has happened. :)
Personally, I do not think it is the end of the world for people to opt
into the "automatically fetch and respect config" method for certain
repositories (and that's why I wrote include.ref support a while ago).
It's a security tradeoff that the user may want to make. But I also
respect the argument that we should not be endorsing risky behavior by
advertising such a feature (especially when the risk is quite subtle, as
many users may not realize that git config can execute arbitrary code).
> I've just started thinking about how to design something that will
> allow us to share configuration elegantly [1]. Essentially, the
> metadata repository will consist of *.layout files, one for each
> repository to clone, containing the .git/config to write after cloning
> that repository. So, a git.layout might look like:
>
> [layout]
> directory = git
> [remote "origin"]
> url = git://github.com/git/git
> [remote "ram"]
> url = git@github.com:artagnon/git
> [remote "junio"]
> url = git://github.com/gitster/git
>
> As you can see the [layout] is a special section which will tell our
> fetcher where to place the repository. Everything else is meant to be
> inserted into the repository's .git/config. However, I can foresee a
> problem in scaling: when I ask a specific directory like a/b/c to be
> populated (equivalent of repo sync `a/b/c`), it'll have to parse the
> layout.directory variable of all the .layout files, and this can be
> slow. So, maybe we should have a special _manifest.layout listing all
> the paths?
>
> Further, I see this as a way to work with projects that would
> otherwise require nested submodules like the Android project. What do
> you think?
Yeah, reading your layout description, this is less about git config in
particular, and more about managing hierarchies of repos. Which I think
is a fine thing to do, and is a sensible place to put config management
(since you are probably executing arbitrary code as part of the layout
tool anyway). But I don't have a real opinion on the design of such a
tool. I have used repo only once or twice to deal with Android. For my
own menagerie of small repos, I have a hacky custom tool that is mostly
about deciding when there are items to be committed, pushed, or fetched
in each repo; I never found the need to handle git config at all.
-Peff
^ permalink raw reply
* random server hacks on top of git
From: Jeff King @ 2013-03-18 12:12 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git
[Re-titled, as we are off-topic from the original patch series]
On Sun, Mar 17, 2013 at 05:38:59PM +0100, René Scharfe wrote:
> Am 17.03.2013 06:40, schrieb Jeff King:
> >We do have the capability to roll out to one or a few of our servers
> >(the granularity is not 0.2%, but it is still small). I'm going to try
> >to keep us more in sync with upstream git, but I don't know if I will
> >get to the point of ever deploying "master" or "next", even for a small
> >portion of the population. We are accumulating more hacks[1] on top of
> >git, so it is not just "run master for an hour on this server"; I have
> >to actually merge our fork.
>
> Did you perhaps intend to list these hacks in a footnote or link to a
> repository containing them? (I can't find the counterpart of that
> [1].)
I was actually just going to say "some of which are gross hacks that
will never see the light of day, some of which have already gone
upstream, and some of which I am planning on submitting upstream".
But since I happened to be cataloguing them recently, here is the list
of things that have not yet gone upstream. If anybody is interested in
a particular topic, I'm happy to discuss and/or prioritize moving it
forward.
- blame-tree; re-rolled from my submission last year to build on top
of the revision machinery, handle merges sanely, etc. Mostly this
needs documentation and a clean-up of the output format (which is
very utilitarian, but probably should share output with git-blame).
- diff --max-depth; this is a requirement to do blame-tree efficiently
if you want to do GitHub-style listings (you must recurse to find
the history of some/subdir, but you do not want to recurse past that
for efficiency reasons). This is hung up on two things:
1. It does not integrate with the pathspec max-depth code, because
we do not use struct pathspec in the tree diff (but I think
Duy's patches are changing that).
2. My definition of --max-depth is subtly different from that of
"git grep". But I think mine is more useful, and I haven't
decided how to reconcile it.
- share ref selection code between "git branch", "git tag", and "git
for-each-ref". This includes cleaning up the "tag --contains" code
to be safer for general use (so that "branch --contains" can benefit
from the speedup), and then getting the same options for all three
commands (tag doesn't know about --merged, and for-each-ref
doesn't know about --contains or --merged).
- receive.maxsize; index-pack will happily spool data to disk
forever, and you never even get a chance to make a policy decision
like "hey, this is too big". This patch lets index-pack cut off the
client after a certain number of bytes. It's not elegant because
the cutoff transfer is not resumable, but we use it is as a
last-ditch for DoS protection (the client can reconnect and send
more, of course, but at that point we have the opportunity to make
external policy decisions like locking their account). Not sure if
other sites would want this or not.
- receive.advertisealternates; basically turn off ".have"
advertisement. Some of our alternates networks are so huge that
the cost of collecting all of the alternate refs is very high (even
though it can save some transfer bandwidth). Not sure if other
sites want this or not (and I think it would be more elegant to
have a small static set of common refs that people build off of,
and advertise those. e.g., if you fork rails/rails, then we should
advertise rails/rails/refs/heads/master as a ".have", but not
anybody else's fork).
- receive.hiderefs; this is going to become redundant with Junio's
implementation
- an audit reflog; we keep a reflog for all refs at the root of the
repository. It differs from a regular reflog in that:
1. It never expires.
2. It is not part of reachability analysis.
3. It includes the refname for each entry, so you can see
deletions.
It's mostly useful for forensics when somebody has screwed up
their repository (or we're chasing down a git bug; it helped me
find the pack-refs race recently). Probably too GitHub-specific
for other people to want it (especially because it grows without
bound).
- statistics instrumentation; we keep counters for various things in
code (e.g., which phase of protocol upload-pack is in, how many
bytes sent, etc) and expose them in a few ways. One is over a
socket to run a "top"-like interface. Another is to tweak the argv
array of the process so that "ps" shows the process state. I think
it would be useful to other people running git servers, but the
code is currently quite nasty and invasive. I have a
work-in-progress to clean it up, but it's got a ways to go.
- hacks to set niceness and io-priority; this should be done by a
wrapper, but in our case it was simpler to catch all processes by
just building it into git. Too gross to go upstream.
- ignore some fsck warnings under transfer.fsckobjects; some of them
are annoyingly common when people pull old history from an
existing project and try to push it back up. It's not indicative
of a new bug in an implementation, but we have to live with the
broken history forever (e.g., zero-padded modes in trees).
-Peff
^ permalink raw reply
* Re: [PATCH/RFC] http_init: only initialize SSL for https
From: Erik Faye-Lund @ 2013-03-18 12:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Daniel Stenberg, Antoine Pelisse, Jeff King, Johannes Schindelin,
git, msysgit
In-Reply-To: <CABPQNSasFV-vZSMygu16xc-C2d3jTt7mtzFsYQyNUhS5jL-EoQ@mail.gmail.com>
On Mon, Mar 18, 2013 at 11:38 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Sun, Mar 17, 2013 at 11:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Daniel Stenberg <daniel@haxx.se> writes:
>>
>>> On Sun, 17 Mar 2013, Antoine Pelisse wrote:
>>>
>>>>> With redirects taken into account, I can't think of any really good way
>>>>> around avoiding this init...
>>>>
>>>> Is there any way for curl to initialize SSL on-demand ?
>>>
>>> Yes, but not without drawbacks.
>>>
>>> If you don't call curl_global_init() at all, libcurl will notice that
>>> on first use and then libcurl will call global_init by itself with a
>>> default bitmask.
>>>
>>> That automatic call of course will prevent the application from being
>>> able to set its own bitmask choice, and also the global_init function
>>> is not (necessarily) thread safe while all other libcurl functions are
>>> so the internal call to global_init from an otherwise thread-safe
>>> function is unfortunate.
>>
>> So in short, unless you are writing a custom application to talk to
>> servers that you know will never redirect you to HTTPS, passing
>> custom masks such as ALL&~SSL to global-init is not going to be a
>> valid optimization.
>>
>> I think that is a reasonable API; your custom application may want
>> to go around your intranet servers all of which serve their status
>> over plain HTTP, and it is a valid optimization to initialize the
>> library with ALL&~SSL. It is just that such an optimization does
>> not apply to us---we let our users go to random hosts we have no
>> control over, and they may redirect us in ways we cannot anticipate.
>>
>
> I wonder. Our libcurl is build with "-winssl" (USE_WINDOWS_SSPI=1), it
> seems. Perhaps switching to openssl (which we already have libraries
> for) would make the init-time better?
It does indeed. So this is probably a better solution, and is
something we're considering doing in Git for Windows anyway (for a
different reason). Thanks for all the feed-back!
^ permalink raw reply
* Re: [PATCH 4/6] introduce a commit metapack
From: Jeff King @ 2013-03-18 12:20 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git, Shawn O. Pearce
In-Reply-To: <CACsJy8CPXFhUYz2f1wuxJvqwknJr5VFNFrs3b_4pS14cxf=3Wg@mail.gmail.com>
On Sun, Mar 17, 2013 at 08:21:13PM +0700, Nguyen Thai Ngoc Duy wrote:
> On Thu, Jan 31, 2013 at 6:06 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> > On Wed, Jan 30, 2013 at 09:16:29PM +0700, Duy Nguyen wrote:
> >> Perhaps we could store abbrev sha-1 instead of full sha-1. Nice
> >> space/time trade-off.
> >
> > Following the on-disk format experiment yesterday, I changed the
> > format to:
> >
> > - a list a _short_ SHA-1 of cached commits
> > ..
> >
> > The length of SHA-1 is chosen to be able to unambiguously identify any
> > cached commits. Full SHA-1 check is done after to catch false
> > positives. For linux-2.6, SHA-1 length is 6 bytes, git and many
> > moderate-sized projects are 4 bytes.
>
> And if we are going to create index v3, the same trick could be used
> for the sha-1 table in the index. We use the short sha-1 table for
> binary search and put the rest of sha-1 in a following table (just
> like file offset table). The advantage is a denser search space, about
> 1/4-1/3 the size of full sha-1 table.
You can make it even smaller at some (potential) run-time cost.
Keep in mind you are just repeating information that is in the full sha1
list in the index. So you could store a fixed-size offset into that list
(e.g., 32-bit), and then instead of comparing sha1s during a binary
search, you would dereference the offset to the real sha1s and compare
those.
The run-time cost is not any worse in a big-O sense, but your cache
locality is much worse (you hit a second random page for each sha1
comparison), which might be noticeable. You'd have to benchmark to see
how big an impact.
-Peff
^ permalink raw reply
* Re: building git ; need suggestion
From: Joydeep Bakshi @ 2013-03-18 12:24 UTC (permalink / raw)
To: Magnus Bäck; +Cc: Fredrik Gustafsson, git
In-Reply-To: <C8080BF5-DC87-421D-97A1-DF5CF403A03A@infoservices.in>
I'm closer to my requirement. I have found gitweb simply provide a GUI for history check
and code comparison. And the git itself is good enough to do the ACL stuff with hooks.
I already have the following code to deploy the push into its work-tree
===========================
#!/bin/bash
while read oldrev newrev ref
do
branch=`echo $ref | cut -d/ -f3`
if [ "master" == "$branch" ]; then
git --work-tree=/path/under/root/dir/live-site/ checkout -f $branch
echo 'Changes pushed live.'
fi
if [ "dev" == "$branch" ]; then
git --work-tree=/path/under/root/dir/dev-site/ checkout -f $branch
echo 'Changes pushed to dev.'
fi
done
=========================
This code can be extended for as many branches as you have.
I now need a mechanism to restrict the user to it's own branch so that user can't push into
any other branch in mistake.
Say I have
master branch -> only admin user can push here.
dev branch -> only user dev1 , dev2 and master can push here.
testing branch -> only user test1 and test2 can push here.
I think this can also be done with pre-receive hook. Any suggestion on the hook design is
welcome. Also this can be implemented on the above hook or in a separate hook.
A separate hook is better due to maintainability and then I need to call multiple
pre-receive hook. Please suggest.
Thanks
On 18-Mar-2013, at 11:14 AM, Joydeep Bakshi <joydeep.bakshi@infoservices.in> wrote:
>
> On 15-Mar-2013, at 6:44 PM, Magnus Bäck <baeck@google.com> wrote:
>>>
>>
>> Right, but that's R/W permissions. Almost any piece of Git hosting
>> software supports restriction of pushes. Discriminating *read* access
>> between developers and maintenance people sounds like a disaster if it's
>> the same organisation.
>
> Just restriction on push access is what required.
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] read-cache: avoid memcpy in expand_name_field in index v4
From: Nguyễn Thái Ngọc Duy @ 2013-03-18 12:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
perf reports memcpy at the the 6th position [1] in "git status -uno"
using index v4, and strbuf_remove() in expand_name_field() accounts
for 25% of that. What we need here is a simple string cut and a
cheaper strbuf_setlen() should be enough. After this change, memcpy
drops down to the 13th position [2] and is dominated by
read_index_from.
[1] before
+ 15.74% git git [.] blk_SHA1_Block
+ 13.22% git [kernel.kallsyms] [k] link_path_walk
+ 10.91% git [kernel.kallsyms] [k] __d_lookup
+ 8.17% git [kernel.kallsyms] [k] strncpy_from_user
+ 4.75% git [kernel.kallsyms] [k] memcmp
+ 2.42% git libc-2.11.2.so [.] memcpy
[2] after
+ 16.30% git git [.] blk_SHA1_Block
+ 13.43% git [kernel.kallsyms] [k] link_path_walk
+ 11.45% git [kernel.kallsyms] [k] __d_lookup
+ 8.73% git [kernel.kallsyms] [k] strncpy_from_user
+ 5.14% git [kernel.kallsyms] [k] memcmp
+ 2.29% git [kernel.kallsyms] [k] do_lookup
+ 2.21% git libc-2.11.2.so [.] 0x6daf6
+ 1.98% git [kernel.kallsyms] [k] _atomic_dec_and_lock
+ 1.98% git [kernel.kallsyms] [k] _raw_spin_lock
+ 1.86% git [kernel.kallsyms] [k] acl_permission_check
+ 1.61% git [kernel.kallsyms] [k] kmem_cache_free
+ 1.59% git git [.] unpack_trees
+ 1.47% git libc-2.11.2.so [.] memcpy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I was after something else when I noticed this. Seems like a simple
and safe change.
read-cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index 827ae55..8c443aa 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1354,7 +1354,7 @@ static unsigned long expand_name_field(struct strbuf *name, const char *cp_)
if (name->len < len)
die("malformed name field in the index");
- strbuf_remove(name, name->len - len, len);
+ strbuf_setlen(name, name->len - len);
for (ep = cp; *ep; ep++)
; /* find the end */
strbuf_add(name, cp, ep - cp);
--
1.8.2.83.gc99314b
^ permalink raw reply related
* [PATCH 0/4] Support triangular workflows
From: Ramkumar Ramachandra @ 2013-03-18 13:16 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King
Hi,
This series follows up a previous discussion with Junio and Jeff [1].
It attempts to support the triangular workflow, where the remote
you're fetching from is not the same as the remote you're pushing to.
`remote.<name>.pushurl` has already been discussed, and deemed as a
poor solution to the problem [2].
[1/4] is a minor cleanup patch to make other patches consistent with
the existing style.
[2/4] introduces the infrastructure needed to allow [3/4] and [4/4] to
be simple configuration-adding patches.
[3/4] and [4/4] add the proposed configuration options. They're very
simple patches, but the documentation is not so simple: I've
documented all the side-effects of the other configuration option in
each configuration option, to give the reader a comprehensive picture
when reading one configuration option.
I've put off implementing remote.default corresponding to
remote.pushdefault, as Jeff suggested in [1], because it's currently
not an itch; apart from the obvious symmetry, I don't know what
purpose it serves: why would anyone want to fetch from a remote other
than origin by default? Why wouldn't they simply swap that remote's
name with "origin"? However, it's a nice thing to have for symmetry,
and it should be trivial to implement: any interested person is
welcome to pick it up.
The series works as expected, and all tests pass.
Thanks for reading.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/215763
[2]: http://thread.gmane.org/gmane.comp.version-control.git/215702/focus=215717
Ramkumar Ramachandra (4):
remote.c: simply a bit of code using git_config_string()
remote.c: introduce a way to have different remotes for fetch/ push
remote.c: introduce remote.pushdefault
remote.c: introduce branch.<name>.pushremote
Documentation/config.txt | 23 ++++++++++++++++---
builtin/push.c | 2 +-
remote.c | 60 +++++++++++++++++++++++++++++++++++-------------
remote.h | 1 +
4 files changed, 66 insertions(+), 20 deletions(-)
--
1.8.2
^ permalink raw reply
* [PATCH 1/4] remote.c: simply a bit of code using git_config_string()
From: Ramkumar Ramachandra @ 2013-03-18 13:16 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1363612575-7340-1-git-send-email-artagnon@gmail.com>
A small segment where handle_config() parses the branch.remote
configuration variable can be simplified using git_config_string().
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
remote.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index e53a6eb..45b69d6 100644
--- a/remote.c
+++ b/remote.c
@@ -356,9 +356,7 @@ static int handle_config(const char *key, const char *value, void *cb)
return 0;
branch = make_branch(name, subkey - name);
if (!strcmp(subkey, ".remote")) {
- if (!value)
- return config_error_nonbool(key);
- branch->remote_name = xstrdup(value);
+ git_config_string(&branch->remote_name, key, value);
if (branch == current_branch) {
default_remote_name = branch->remote_name;
explicit_default_remote_name = 1;
--
1.8.2
^ permalink raw reply related
* [PATCH 3/4] remote.c: introduce remote.pushdefault
From: Ramkumar Ramachandra @ 2013-03-18 13:16 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1363612575-7340-1-git-send-email-artagnon@gmail.com>
This new configuration variable defines the default remote to push to,
and overrides `branch.<name>.remote` for all branches. It is useful
in the typical triangular-workflow setup, where the remote you're
fetching from is different from the remote you're pushing to.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/config.txt | 13 ++++++++++---
remote.c | 4 ++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bbba728..8ddd0fd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -723,9 +723,12 @@ branch.autosetuprebase::
This option defaults to never.
branch.<name>.remote::
- When in branch <name>, it tells 'git fetch' and 'git push' which
- remote to fetch from/push to. It defaults to `origin` if no remote is
- configured. `origin` is also used if you are not on any branch.
+ When on branch <name>, it tells 'git fetch' and 'git push'
+ which remote to fetch from/push to. The remote to push to
+ may be overriden with `remote.pushdefault` (for all branches).
+ If no remote is configured, or if you are not on any branch,
+ it defaults to `origin` for fetching and `remote.pushdefault`
+ for pushing.
branch.<name>.merge::
Defines, together with branch.<name>.remote, the upstream branch
@@ -1894,6 +1897,10 @@ receive.updateserverinfo::
If set to true, git-receive-pack will run git-update-server-info
after receiving data from git-push and updating refs.
+remote.pushdefault::
+ The remote to push to by default. Overrides
+ `branch.<name>.remote` for all branches.
+
remote.<name>.url::
The URL of a remote repository. See linkgit:git-fetch[1] or
linkgit:git-push[1].
diff --git a/remote.c b/remote.c
index 4704404..987edc4 100644
--- a/remote.c
+++ b/remote.c
@@ -350,6 +350,10 @@ static int handle_config(const char *key, const char *value, void *cb)
const char *subkey;
struct remote *remote;
struct branch *branch;
+ if (!prefixcmp(key, "remote.")) {
+ if (!strcmp(key + 7, "pushdefault"))
+ git_config_string(&pushremote_name, key, value);
+ }
if (!prefixcmp(key, "branch.")) {
name = key + 7;
subkey = strrchr(name, '.');
--
1.8.2
^ permalink raw reply related
* [PATCH 4/4] remote.c: introduce branch.<name>.pushremote
From: Ramkumar Ramachandra @ 2013-03-18 13:16 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1363612575-7340-1-git-send-email-artagnon@gmail.com>
This new configuration variable overrides `remote.pushdefault` and
`branch.<name>.remote` for pushes. In a typical triangular-workflow
setup, you would want to set `remote.pushdefault` to specify the
remote to push to for all branches, and use this option to override it
for a specific branch.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/config.txt | 18 ++++++++++++++----
remote.c | 3 +++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8ddd0fd..d0e36e9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -726,9 +726,18 @@ branch.<name>.remote::
When on branch <name>, it tells 'git fetch' and 'git push'
which remote to fetch from/push to. The remote to push to
may be overriden with `remote.pushdefault` (for all branches).
- If no remote is configured, or if you are not on any branch,
- it defaults to `origin` for fetching and `remote.pushdefault`
- for pushing.
+ The remote to push to, for the current branch, may be further
+ overriden by `branch.<name>.pushremote`. If no remote is
+ configured, or if you are not on any branch, it defaults to
+ `origin` for fetching and `remote.pushdefault` for pushing.
+
+branch.<name>.pushremote::
+ When on branch <name>, it overrides `branch.<name>.remote`
+ when pushing. It also overrides `remote.pushdefault` when
+ pushing from branch <name>. In a typical triangular-workflow
+ setup, you would want to set `remote.pushdefault` to specify
+ the remote to push to for all branches, and use this option to
+ override it for a specific branch.
branch.<name>.merge::
Defines, together with branch.<name>.remote, the upstream branch
@@ -1899,7 +1908,8 @@ receive.updateserverinfo::
remote.pushdefault::
The remote to push to by default. Overrides
- `branch.<name>.remote` for all branches.
+ `branch.<name>.remote` for all branches, and is overriden by
+ `branch.<name>.pushremote` for specific branches.
remote.<name>.url::
The URL of a remote repository. See linkgit:git-fetch[1] or
diff --git a/remote.c b/remote.c
index 987edc4..a4d3d22 100644
--- a/remote.c
+++ b/remote.c
@@ -366,6 +366,9 @@ static int handle_config(const char *key, const char *value, void *cb)
default_remote_name = branch->remote_name;
explicit_default_remote_name = 1;
}
+ } else if (!strcmp(subkey, ".pushremote")) {
+ if (branch == current_branch)
+ git_config_string(&pushremote_name, key, value);
} else if (!strcmp(subkey, ".merge")) {
if (!value)
return config_error_nonbool(key);
--
1.8.2
^ permalink raw reply related
* [PATCH 2/4] remote.c: introduce a way to have different remotes for fetch/ push
From: Ramkumar Ramachandra @ 2013-03-18 13:16 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1363612575-7340-1-git-send-email-artagnon@gmail.com>
Currently, do_push() in push.c calls remote_get(), which gets the
configured remote for fetching and pushing. Replace this call with a
call to pushremote_get() instead, a new function that will return the
remote configured specifically for pushing. This function tries to
work with the string pushremote_name, before falling back to the
codepath of remote_get(). This patch has no visible impact, but
serves to enable future patches to introduce configuration variables
to set this variable.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/push.c | 2 +-
remote.c | 49 ++++++++++++++++++++++++++++++++++++-------------
remote.h | 1 +
3 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 42b129d..d447a80 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags)
static int do_push(const char *repo, int flags)
{
int i, errs;
- struct remote *remote = remote_get(repo);
+ struct remote *remote = pushremote_get(repo);
const char **url;
int url_nr;
diff --git a/remote.c b/remote.c
index 45b69d6..4704404 100644
--- a/remote.c
+++ b/remote.c
@@ -48,6 +48,7 @@ static int branches_nr;
static struct branch *current_branch;
static const char *default_remote_name;
+static const char *pushremote_name;
static int explicit_default_remote_name;
static struct rewrites rewrites;
@@ -669,20 +670,9 @@ static int valid_remote_nick(const char *name)
return !strchr(name, '/'); /* no slash */
}
-struct remote *remote_get(const char *name)
+static struct remote *remote_get_1(const char *name, int name_given)
{
- struct remote *ret;
- int name_given = 0;
-
- read_config();
- if (name)
- name_given = 1;
- else {
- name = default_remote_name;
- name_given = explicit_default_remote_name;
- }
-
- ret = make_remote(name, 0);
+ struct remote *ret = make_remote(name, 0);
if (valid_remote_nick(name)) {
if (!valid_remote(ret))
read_remotes_file(ret);
@@ -698,6 +688,39 @@ struct remote *remote_get(const char *name)
return ret;
}
+struct remote *remote_get(const char *name)
+{
+ int name_given = 0;
+
+ read_config();
+ if (name)
+ name_given = 1;
+ else {
+ name = default_remote_name;
+ name_given = explicit_default_remote_name;
+ }
+ return remote_get_1(name, name_given);
+}
+
+struct remote *pushremote_get(const char *name)
+{
+ int name_given = 0;
+
+ read_config();
+ if (name)
+ name_given = 1;
+ else {
+ if (pushremote_name) {
+ name = pushremote_name;
+ name_given = 1;
+ } else {
+ name = default_remote_name;
+ name_given = explicit_default_remote_name;
+ }
+ }
+ return remote_get_1(name, name_given);
+}
+
int remote_is_configured(const char *name)
{
int i;
diff --git a/remote.h b/remote.h
index 251d8fd..99a437f 100644
--- a/remote.h
+++ b/remote.h
@@ -51,6 +51,7 @@ struct remote {
};
struct remote *remote_get(const char *name);
+struct remote *pushremote_get(const char *name);
int remote_is_configured(const char *name);
typedef int each_remote_fn(struct remote *remote, void *priv);
--
1.8.2
^ permalink raw reply related
* Re: [PATCH] git-p4: support exclusively locked files
From: Danny Thomas @ 2013-03-18 13:26 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git@vger.kernel.org
In-Reply-To: <20130317200437.GA29115@padd.com>
Interesting. 'Implementing sitewide pessimistic locking with p4 typemap',
http://www.perforce.com/perforce/doc.current/manuals/p4sag/03_superuser.htm
l seems to suggest this is all that's needed. I believe we're using the
default configuration, the exclusive lock on all files behaviour was
researching the exclusive locking problem,
http://ericlathrop.com/2012/12/how-to-set-up-git-p4-in-windows/, so I
thought I'd mention it.
You might be onto something w/ fstat, it doesn't include the exclusive
indicator:
... type binary+l
Latest P4 client, and fairly recent server build:
P4/DARWIN90X86_64/2012.2/536738 (2012/10/16)
P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)
Danny
On 17/03/2013 20:04, "Pete Wyckoff" <pw@padd.com> wrote:
>Danny.Thomas@blackboard.com wrote on Wed, 13 Mar 2013 13:51 -0400:
>> By default, newly added binary files are exclusively locked by Perforce:
>>
>> 'add default change (binary+l) *exclusive*'
>>
>> This results in a 'Could not determine file type' error as the regex
>> expects
>> the line to end after the file type matching group. Some repositories
>>are
>> also configured to always require exclusive locks, so may be a problem
>>for
>> all revisions in some cases.
>
>Can you explain how to configure p4d to default everything to
>binary+l? Also, what version are you using ("p4 info")? I'm
>trying to write a test case for this.
>
>I did find a way to play with typemap to get +l, as:
>
> ( p4 typemap -o ; printf "\tbinary+l\t//.../bash*" ) | p4 typemap -i
>
>With this, the 2011.1 here just says:
>0
> tic-git-test$ p4 opened bash
> //depot/bash#1 - add default change (binary+l)
>
>I've not been able to make it say " *exclusive*" too.
>
>> result = p4_read_pipe(["opened", wildcard_encode(file)])
>> - match = re.match(".*\((.+)\)\r?$", result)
>> + match = re.match(".*\((.+)\)(?:.+)?\r?$", result)
>
>I think this whole function would be less brittle
>using p4's "-G" output, like:
>
> d = p4Cmd(["fstat", "-T", "type", wildcard_encode(file)])
> # some error checking
> return d['type']
>
>But I'm curious if your p4d includes " *exclusive*" in the
>type there too, in which case we'll have to strip it.
>
>Thanks for starting the patch on this. It's good if we can keep
>others from running into the same problem.
>
> -- Pete
This email and any attachments may contain confidential and proprietary information of Blackboard that is for the sole use of the intended recipient. If you are not the intended recipient, disclosure, copying, re-distribution or other use of any of this information is strictly prohibited. Please immediately notify the sender and delete this transmission if you received this email in error.
^ permalink raw reply
* Re: Re: [PATCH v2 4/4] teach config parsing to read from strbuf
From: Heiko Voigt @ 2013-03-18 14:18 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, Jens Lehmann, Ramsay Jones
In-Reply-To: <20130314073913.GA7024@sigill.intra.peff.net>
Hi Peff,
On Thu, Mar 14, 2013 at 03:39:14AM -0400, Jeff King wrote:
> On Thu, Mar 14, 2013 at 03:10:46AM -0400, Jeff King wrote:
>
> > I looked into this a little. The first sticking point is that
> > git_config_with_options needs to support the alternate source. Here's a
> > sketch of what I think the solution should look like, on top of your
> > patches.
>
> Here it is again, with two changes:
>
> 1. Rather than handling the blob lookup inline in
> git_config_with_options, it adds direct functions for reading
> config from blob sha1s and blob references. I think this should
> make it easier to reuse when you are trying to read .gitmodules
> from C code.
>
> 2. It adds some basic tests.
>
> I'll leave it here for tonight. The next step would be to rebase it on
> your modified series (in particular, I think git_config_from_strbuf
> should become git_config_from_buf, which will impact this).
>
> Feel free to use, pick apart, rewrite, or discard as you see fit for
> your series.
Sorry for the late reply, I was not online during the last days. Thanks
a lot for this. I will use it in the next iteration of the series.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH 0/4] Support triangular workflows
From: Jeff King @ 2013-03-18 14:25 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <1363612575-7340-1-git-send-email-artagnon@gmail.com>
On Mon, Mar 18, 2013 at 06:46:11PM +0530, Ramkumar Ramachandra wrote:
> I've put off implementing remote.default corresponding to
> remote.pushdefault, as Jeff suggested in [1], because it's currently
> not an itch; apart from the obvious symmetry, I don't know what
> purpose it serves: why would anyone want to fetch from a remote other
> than origin by default? Why wouldn't they simply swap that remote's
> name with "origin"? However, it's a nice thing to have for symmetry,
> and it should be trivial to implement: any interested person is
> welcome to pick it up.
Yeah, I agree that it does not have much point, aside from people who
have an unreasonable aversion to using the word "origin". There was a
series posted last summer to add remote.default:
http://article.gmane.org/gmane.comp.version-control.git/201065
It ended up stalled and never got merged. I think the main impetus was
that "git clone -o foo" should leave "foo" in remote.default (of course,
that still leaves unanswered why anyone would really want to use "-o
foo" in the first place).
I think the symmetry makes some sense, but I also think it can come
later if somebody wants it.
> Documentation/config.txt | 23 ++++++++++++++++---
> builtin/push.c | 2 +-
> remote.c | 60 +++++++++++++++++++++++++++++++++++-------------
> remote.h | 1 +
> 4 files changed, 66 insertions(+), 20 deletions(-)
No new tests?
-Peff
^ permalink raw reply
* Re: [PATCH 0/4] Support triangular workflows
From: Ramkumar Ramachandra @ 2013-03-18 14:28 UTC (permalink / raw)
To: Jeff King; +Cc: Git List, Junio C Hamano
In-Reply-To: <20130318142526.GA23075@sigill.intra.peff.net>
Jeff King wrote:
> On Mon, Mar 18, 2013 at 06:46:11PM +0530, Ramkumar Ramachandra wrote:
>> Documentation/config.txt | 23 ++++++++++++++++---
>> builtin/push.c | 2 +-
>> remote.c | 60 +++++++++++++++++++++++++++++++++++-------------
>> remote.h | 1 +
>> 4 files changed, 66 insertions(+), 20 deletions(-)
>
> No new tests?
Honestly, it slipped my mind. Will write it now.
Thanks for the reminder.
^ permalink raw reply
* Re: [PATCH 2/4] remote.c: introduce a way to have different remotes for fetch/ push
From: Jeff King @ 2013-03-18 14:31 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <1363612575-7340-3-git-send-email-artagnon@gmail.com>
On Mon, Mar 18, 2013 at 06:46:13PM +0530, Ramkumar Ramachandra wrote:
> +struct remote *remote_get(const char *name)
> +{
> + int name_given = 0;
> +
> + read_config();
> + if (name)
> + name_given = 1;
> + else {
> + name = default_remote_name;
> + name_given = explicit_default_remote_name;
> + }
> + return remote_get_1(name, name_given);
> +}
> +
> +struct remote *pushremote_get(const char *name)
> +{
> + int name_given = 0;
> +
> + read_config();
> + if (name)
> + name_given = 1;
> + else {
> + if (pushremote_name) {
> + name = pushremote_name;
> + name_given = 1;
> + } else {
> + name = default_remote_name;
> + name_given = explicit_default_remote_name;
> + }
> + }
> + return remote_get_1(name, name_given);
> +}
Can we get rid of this duplication by having remote_get_1 take a
service-specific default argument? And then each service calls it like:
struct remote *remote_get(const char *name)
{
read_config();
return remote_get_1(name, NULL);
}
struct remote *pushremote_get(const char *name)
{
read_config();
return remote_get_1(name, pushremote_name);
}
and all of the name_given junk can stay in remote_get_1. And adding
"remote.default" would just be a matter of changing that NULL in
remote_get.
-Peff
^ permalink raw reply
* Re: [PATCH 0/4] Support triangular workflows
From: Jeff King @ 2013-03-18 14:32 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0=D6JmocJJcLJTV6z-TcyuJL35cDpozaa6t64V_W2KSRQ@mail.gmail.com>
On Mon, Mar 18, 2013 at 07:58:23PM +0530, Ramkumar Ramachandra wrote:
> Jeff King wrote:
> > On Mon, Mar 18, 2013 at 06:46:11PM +0530, Ramkumar Ramachandra wrote:
> >> Documentation/config.txt | 23 ++++++++++++++++---
> >> builtin/push.c | 2 +-
> >> remote.c | 60 +++++++++++++++++++++++++++++++++++-------------
> >> remote.h | 1 +
> >> 4 files changed, 66 insertions(+), 20 deletions(-)
> >
> > No new tests?
>
> Honestly, it slipped my mind. Will write it now.
>
> Thanks for the reminder.
Thanks. Other than the suggestion I made on 2/4, I do not see anything
wrong with the series.
-Peff
^ permalink raw reply
* Re: [PATCH 2/4] remote.c: introduce a way to have different remotes for fetch/ push
From: Ramkumar Ramachandra @ 2013-03-18 14:56 UTC (permalink / raw)
To: Jeff King; +Cc: Git List, Junio C Hamano
In-Reply-To: <20130318143121.GB23075@sigill.intra.peff.net>
Jeff King wrote:
> On Mon, Mar 18, 2013 at 06:46:13PM +0530, Ramkumar Ramachandra wrote:
>
>> +struct remote *remote_get(const char *name)
>> +{
>> + int name_given = 0;
>> +
>> + read_config();
>> + if (name)
>> + name_given = 1;
>> + else {
>> + name = default_remote_name;
>> + name_given = explicit_default_remote_name;
>> + }
>> + return remote_get_1(name, name_given);
>> +}
>> +
>> +struct remote *pushremote_get(const char *name)
>> +{
>> + int name_given = 0;
>> +
>> + read_config();
>> + if (name)
>> + name_given = 1;
>> + else {
>> + if (pushremote_name) {
>> + name = pushremote_name;
>> + name_given = 1;
>> + } else {
>> + name = default_remote_name;
>> + name_given = explicit_default_remote_name;
>> + }
>> + }
>> + return remote_get_1(name, name_given);
>> +}
>
> Can we get rid of this duplication by having remote_get_1 take a
> service-specific default argument? And then each service calls it like:
>
> struct remote *remote_get(const char *name)
> {
> read_config();
> return remote_get_1(name, NULL);
> }
>
> struct remote *pushremote_get(const char *name)
> {
> read_config();
> return remote_get_1(name, pushremote_name);
> }
Thanks for the dose of sanity. While at it, why not move
read_config() to remote_get_1() as well?
^ permalink raw reply
* Re: [PATCH 2/4] remote.c: introduce a way to have different remotes for fetch/ push
From: Jeff King @ 2013-03-18 14:58 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0mc3HN4jT24QKP2Bt49fPtSd4=3wO0xFA7-4gt2n7ih-g@mail.gmail.com>
On Mon, Mar 18, 2013 at 08:26:46PM +0530, Ramkumar Ramachandra wrote:
> > Can we get rid of this duplication by having remote_get_1 take a
> > service-specific default argument? And then each service calls it like:
> >
> > struct remote *remote_get(const char *name)
> > {
> > read_config();
> > return remote_get_1(name, NULL);
> > }
> >
> > struct remote *pushremote_get(const char *name)
> > {
> > read_config();
> > return remote_get_1(name, pushremote_name);
> > }
>
> Thanks for the dose of sanity. While at it, why not move
> read_config() to remote_get_1() as well?
Because it sets pushremote_name, and therefore you would just be passing
NULL if read_config has not been run yet. But if you made it:
return remote_get_1(name, &pushremote_name);
that would work.
-Peff
^ permalink raw reply
* Re: [PATCH v2 4/4] pack-refs: add fully-peeled trait
From: Junio C Hamano @ 2013-03-18 15:12 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Jeff King, git
In-Reply-To: <51468615.70503@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>
> and ACK for the whole series, once Junio's points are addressed.
>
> Regarding Junio's readability suggestion: I agree that his versions are
> a bit more readable, albeit at the expense of having to evaluate a bit
> more logic for each reference rather than just once when the header line
> is handled. So I don't have a preference either way.
The way the conditional is written, in the longer term we
will almost always compare "peeled == PEELED_FULLY", and otherwise
we will do the same !prefixcmp(refs/tags/), so I do not think there
is "more logic" that matters compared to the original.
Thanks, both; will replace what was queued with "SQUASH???".
^ permalink raw reply
* Re: Git build fails on `make`, undefined references in libcrypto.a.
From: zero modulo @ 2013-03-18 16:04 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git
In-Reply-To: <20130318072933.GC5434@localhost.localdomain>
On Mon, Mar 18, 2013 at 1:29 AM, Konstantin Khomoutov
<kostix+git@007spb.ru> wrote:
> FYI, I've already tried to answer this exact question [1] with no
> comments from the OP.
>
> 1. http://serverfault.com/a/488604/118848
It is I who posted that question. :P
I haven't made any comments yet because this issue is still a work in
progress. I re-compiled OpenSSL taking into consideration that libdl.a
probably wasn't linked properly to libcrypto.a, and have also tried
re-compiling Git afterward, but with the same errors. I have also
created a .conf file for `ld.so`, and ran `sudo ldconfig` which solved
some other issues I was having. Running `ldd msgfmt` revealed that
run-time libraries were not being found, and after running `sudo
ldconfig`, `ldd msgfmt` then showed the libgettext .so was linked. So,
I solved some of my issues, and if there was an issue with statically
linked libraries not being found, as in this case, then, I believe
re-compiling OpenSSL with the proper LDFLAGS and CPPFLAGS would have
solved the issue, but they have not.
I'm currently attempting to install GCC 4.7.2, which is having some
other issues with texinfo 5.1. I can't find the link, but I someone
said it could be the compiler version... since everything else that
seems like might be the issue isn't fixing it, I'm going to try
re-compiling OpenSSL with GCC 4.7.2 and see how that goes.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox