* Re: svn to git with non-standard layout
From: Frans Klaver @ 2011-11-23 21:19 UTC (permalink / raw)
To: git, Bernd Schubert
In-Reply-To: <jajh7m$it7$1@dough.gmane.org>
On Wed, 23 Nov 2011 20:23:34 +0100, Bernd Schubert
<bernd.schubert@itwm.fraunhofer.de> wrote:
> we just want to migrate from subversion to git, but so far all attempts
> to do that resulted in missing commit information.
What are all attempts? Did you try only git-svn, or did you try tools like
svn2git?
> Any ideas what I still could try to do?
If you haven't tried anything other than git-svn, try svneverever &
svn2git[1] for an actual repository conversion. Or have a look at the
newest kid on the block called SubGit[2][3]. In both I don't have any
experience, but of svn2git I know Qt and KDE have used it successfully to
convert their repositories.
I assume both are somewhat more complex to use and set up than to simply
use git-svn, but I guess it will be worth the effort.
Good luck,
Frans
[1] http://techbase.kde.org/Projects/MoveToGit/UsingSvn2Git
[2] http://subgit.com/
[3] http://thread.gmane.org/gmane.comp.version-control.git/185806
^ permalink raw reply
* Re: [PATCH] builtin-branch: Fix crash on invalid use of --force
From: Junio C Hamano @ 2011-11-23 21:30 UTC (permalink / raw)
To: vfr; +Cc: git
In-Reply-To: <1322029915-18486-1-git-send-email-vfr@lyx.org>
Makes sense and is a nice fix for a regression on 'master'. Thanks.
^ permalink raw reply
* Re: Git ticket / issue tracking ERA: Git shouldn't allow to push a new branch called HEAD
From: Frans Klaver @ 2011-11-23 21:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Paolo Ciarrocchi, Daniele Segato, Git Mailing List, Jeff King,
Scott Chacon
In-Reply-To: <7v4nxudb73.fsf@alter.siamese.dyndns.org>
On Wed, 23 Nov 2011 18:02:08 +0100, Junio C Hamano <gitster@pobox.com>
wrote:
> Frans Klaver <fransklaver@gmail.com> writes:
>
>> The note from the maintainer[1] mentions
>>
>> git://git.kernel.org/pub/scm/git/git.git/
>> git://repo.or.cz/alt-git.git
>> https://github.com/git/git
>> https://code.google.com/p/git-core/
>>
>> I would assume one of those would be a nomination for 'official' repo.
>>
>> Maybe something for Scott C. to address?
>
> As long as the link says "Git source repository" without "the official",
> Scott is doing the right thing. It is just one of the copies that I push
> into, so it may be considered more official than a fork of my history by
> a
> random unknown person.
>
> As Git is distributed, we do not need a single "official" repository. If
> you really want to name one, my private working repository at my home
> machine would be what is closest to one, as patches and pull requests are
> processed there and then the result is pushed out to the above four and a
> few others. But that "official" one is not exposed to the outside world
> ;-)
Since official is a rather unreal term here, let's just drop it. There are
a few repositories that the maintainer currently pushes to, and those are
the most reliable ones to use if you want the latest git vanilla. Other
than that, there's going to be no difference from the next git.git clone.
It might remove some confusion if these repos would be reflected on
websites focusing on git as they are in the maintainers notes, but that's
probably up to the respective webmasters then.
Frans
^ permalink raw reply
* Re: git-bisect working only from toplevel dir
From: Jeff King @ 2011-11-23 21:36 UTC (permalink / raw)
To: Peter Baumann; +Cc: Junio C Hamano, Adam Borowski, git
In-Reply-To: <20111123202643.GB6291@m62s10.vlinux.de>
On Wed, Nov 23, 2011 at 09:26:43PM +0100, Peter Baumann wrote:
> > If we cd_to_toplevel, we can remember the prefix that we started from
> > and cd to it before running the user's command, but there is no
> > guarantee that it actually exists. Maybe that commit should be
> > considered indeterminate then?
> >
>
> Why not simply fail the run with exit(-1)? If the directory doesn't exist
> in an older commit (which I think is not that common) git bisect should
> simply stop and let the user proceed.
The point of "git bisect run" is to run unattended until we reach an
answer. I don't think most people would be happy with it not running to
come to _some_ answer (e.g., imagine checking the results of an
overnight "bisect run" in the morning only to find that it stopped 20
minutes in).
That's why I think just marking the commit as indeterminate would be
better; it jumps over parts of history that omit the directory, and will
generally still come to a good conclusion. If it's possible to get an
answer, that is. It might say "we can't come up with an answer because
all of these commits are not testable". But that tells you something,
too: your bisection test is not a good one.
> And yes, I find the current behaviour to forbid running git bisect from
> a subdirectory slighly annoying and I'm glad somebody took a stab at it.
Agreed. I often bisect by hand with two terminals, doing something like:
[terminal 1]
git bisect start ...
make
[terminal 2]
cd t
./t1234-whatever -v
[terminal 1]
git bisect good|bad
make
[terminal 2]
./t1234-whatever
And then want to type "git bisect good|bad" into terminal 2. Which
doesn't work, of course (yes, in this simple case I could automate the
running of the test script from terminal 1; but often times it is
simpler to just eyeball the output during the bisection).
-Peff
^ permalink raw reply
* Re: git-bisect working only from toplevel dir
From: Jeff King @ 2011-11-23 21:45 UTC (permalink / raw)
To: Adam Borowski; +Cc: git
In-Reply-To: <20111123200920.GA21004@angband.pl>
On Wed, Nov 23, 2011 at 09:09:20PM +0100, Adam Borowski wrote:
> > But from what directory would you expect:
> >
> > git bisect run make
> >
> > to run from? If you use a GNU-ish layout with all of your code in
> > "src/",
>
> In a vast majority of cases the layout remains constant during the whole
> bisection.
Agreed. But you need to think about what happens when it does not. I
think marking the commit as untestable is probably best, with bisect
barfing a reasonable second. Accidentally marking the commit as "bad" is
probably the worst thing we could do. That would produce a subtly wrong
bisection result.
> > Maybe that commit should be considered indeterminate then?
>
> Why? If you're running an automated command, then it will probably fail,
> yeah. I guess most people bisect manually though, so even in repositories
> that do have this problem, there's someone who can test the given commit
> anyway.
If you're not doing "bisect run", then it is a non-issue, no? If you
are bisecting by hand, then "git bisect good|bad" will delete your
working directory, and probably your shell will start complaining, and
an intelligent tester will see what happened. This is only a problem for
automated bisection, which does not have such a tester.
> > I dunno. I haven't thought that hard about it. But I don't think it's
> > quite as simple as just telling bisect it's OK to run from a subdir.
>
> At the very least, generally working with a caveat in corner cases seems to
> be better than outright failing.
To be clear: I think this is a good feature that will help a lot of
people, and I don't think an uncommon corner case should prevent it from
going into git. But I _do_ think we should consider what happens in the
corner cases and at least fail gracefully, rather than produce subtly
wrong results.
-Peff
^ permalink raw reply
* Re: svn to git with non-standard layout
From: Bernd Schubert @ 2011-11-23 21:45 UTC (permalink / raw)
To: Frans Klaver; +Cc: git
In-Reply-To: <op.v5e36lxp0aolir@keputer.lokaal>
On 11/23/2011 10:19 PM, Frans Klaver wrote:
> On Wed, 23 Nov 2011 20:23:34 +0100, Bernd Schubert
> <bernd.schubert@itwm.fraunhofer.de> wrote:
>
>> we just want to migrate from subversion to git, but so far all
>> attempts to do that resulted in missing commit information.
>
> What are all attempts? Did you try only git-svn, or did you try tools
> like svn2git?
We are trying to do this ever since Monday and it included svn2git as
well. The problem with svn2git is that you can provide several tags and
branch paths, but only a single trunk path.
>
>
>> Any ideas what I still could try to do?
>
> If you haven't tried anything other than git-svn, try svneverever &
> svn2git[1] for an actual repository conversion. Or have a look at the
> newest kid on the block called SubGit[2][3]. In both I don't have any
> experience, but of svn2git I know Qt and KDE have used it successfully
> to convert their repositories.
>
> I assume both are somewhat more complex to use and set up than to simply
> use git-svn, but I guess it will be worth the effort.
>
> Good luck,
> Frans
>
> [1] http://techbase.kde.org/Projects/MoveToGit/UsingSvn2Git
> [2] http://subgit.com/
> [3] http://thread.gmane.org/gmane.comp.version-control.git/185806
Thanks, I'm going to read about svneverever and SubGit tomorrow. We will
also try another tool that can convert an svn dump (once we get a dump,
as we don't have easy access to the svn database).
Thanks,
Bernd
^ permalink raw reply
* Re: Possible bug with branch names and case sensitivity
From: Joshua Jensen @ 2011-11-23 22:08 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Jay Soffian, Michael Haggerty, Gerd Knops, git
In-Reply-To: <CACBZZX4qs8-u33bZbrxYS1CrwjTQc=4YOk2SUjtYzL=vc9KYgA@mail.gmail.com>
----- Original Message -----
From: Ævar Arnfjörð Bjarmason
Date: 11/23/2011 1:50 PM
>
>
> Note that Git doesn't only have confusing behavior with refs on
> case-insensitive filesystems. The other day HFS+ users @ work had
> issues because of a case collision in the checked out tree, which
> confused git status et al.
Is core.ignorecase set to true? Is the repository shared with a case
sensitive file system?
I have a patch sitting around for 'git update-index --add' that fixes
some case insensitivity issues, especially when using Git Gui. This
patch complements the core.ignorecase patches I sent in the past.
-Josh
^ permalink raw reply
* Re: git fetch overwriting local tags
From: Jeff King @ 2011-11-23 22:16 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: git, John Kacur
In-Reply-To: <20111123090821.GL19986@pengutronix.de>
On Wed, Nov 23, 2011 at 10:08:21AM +0100, Uwe Kleine-König wrote:
> John and I wondered about git fetch overwriting local tags. I was sure
> enough to claim that git fetch won't overwrite local tags with remote
> tags having the same name. But after John pointed me to
>
> http://www.pythian.com/news/9067/on-the-perils-of-importing-remote-tags-in-git/
>
> I tested that (using Debian's 1.7.7.3) and really, git does overwrite
> local tags.
>
> Here is my test script:
> [...]
> git fetch --tags ../a
> [...]
> Is this intended?
Sort of.
By default, "git fetch" will "auto-follow" tags; if you fetch a commit
which is pointed to by a tag, then git will fetch that tag, too. So
generally, you shouldn't need to specify "--tags" at all, because you
will already be getting the relevant tags.
The "--tags" option, however, is a short-hand for saying "fetch all of
the tags", and is equivalent to providing the refspec:
git fetch ../a refs/tags/*:refs/tags/*
Which of course will update your local tags with similarly-named ones
from the remote. So in that sense, there is no bug, and it is working
as intended; the problem is that the author's intent was not the same as
your intent. :)
I'm not sure why you're using "--tags" in the first place. That might
help us figure out if there's another way to do what you want that is
safer.
That being said, it would be nice if "--tags" wasn't so surprising.
Three things that I think could help are:
1. We usually require a "+" on the refspec (or "--force") to update
non-fast-forward branches. But there is no such safety on tags
(which generally shouldn't be updated at all). Should we at least
be enforcing the same fast-forward rules on tag fetches (or even
something more strict, like forbidding tag update at all unless
forced)?
2. We don't keep a reflog on tags. Generally there's no point. But
it wouldn't be very expensive (since they don't usually change),
and could provide a safety mechanism here.
3. Keeping tags from remotes in separate namespaces, but collating
them at lookup time. This has been discussed, and I think is
generally a fine idea, but nobody has moved forward with code.
-Peff
^ permalink raw reply
* Re: [PATCH] Fix https interactive authentication problem
From: Jeff King @ 2011-11-23 22:51 UTC (permalink / raw)
To: Steinmann Ruedi; +Cc: git@vger.kernel.org, gitster@pobox.com
In-Reply-To: <F6A589D6B10801478E0DE246A9EF187C1BD5E8@MBX12.d.ethz.ch>
On Wed, Nov 23, 2011 at 07:58:02AM +0000, Steinmann Ruedi wrote:
> Cloning a repository over https works fine when the username/password is
> given in the URL. But if it is queried interactively, an error occurs(see below).
> I found that the username/password is not set when a connection is reused.
>
> With this patch, the username/password is set whenever a connection is reused.
>
> Sample output showing the error:
>
> git clone https://n.ethz.ch/student/...
> Cloning into ...
> Username:
> Password:
> error: Unable to get pack file https://n.ethz.ch/student/.../objects/pack/pack-1ced2ebff0c9fc1f07e0c7cc9dd3fc75f6ac6962.pack
> The requested URL returned error: 401
> ...
> error: Fetch failed.
Hmm. What version of git are you using?
We used to always set up the auth information in get_curl_handle, so all
handles had any auth information that we already got. But we removed
that recently in 986bbc0 (http: don't always prompt for password,
2011-11-04), which is in v1.7.8-rc1 and later.
I'm wondering if this was an unintended side effect of that patch.
Usually git will notice the 401 and retry with credentials (requesting
them from the user if necessary). That used to not work for all requests
but I fixed that in 8d677ed (http: retry authentication failures for all
http requests, 2011-07-18), which is also in v1.7.8-rc*.
However, it looks like the packfile-fetching code for dumb http does not
actually go through the usual http_request method, and does not
recognize the 401.
So if you are using v1.7.8-rc*, then I think that is the issue. And your
patch is just undoing what 8d677ed did. But the right fix is to refactor
the packfile-fetching function on top of the other http-fetching code,
so it can get the benefit of that logic.
If you are using an older version of git (pre-8d677ed), then one guess
about what is happening is a race condition with setting up multiple
curl handles:
1. we get curl handle 1 from get_curl_handle, calling
init_curl_http_auth. But we have no username in the url, so we
assume no credentials are needed.
2. we get curl handle 2 from get_curl_handle, as above.
3. we make a request on handle one, get a 401, and ask for
credentials.
4. we make a request for a packfile on handle 2, but we never copied
the auth from step (3) into the curl handle from step (2), and it
fails.
(I haven't looked closely at our usage of curl_multi_*, so I am not 100%
sure that this can even happen. So consider it just a theory).
If this is the case, your fix works because it calls init_curl_http_auth
more often (i.e., before making any request, not just when we create the
handle). But that means it is also undoing what 8d677ed did; we will ask
the user for credentials before actually seeing a 401.
We could still fix this by teaching the packfile-fetching code to
respect 401s. However, it does incur an unnecessary round-trip (we
already know from the other request that credentials are required, but
we don't bother to use them).
So I think the logic you want instead is "if we already have
credentials, use them with this curl slot, but otherwise don't bother
the user". Which would mean splitting the prompting bit out of
init_curl_http_auth.
-Peff
^ permalink raw reply
* Re: [PATCH] run-command.c: Accept EACCES as command not found
From: Frans Klaver @ 2011-11-23 22:55 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Junio C Hamano
In-Reply-To: <CAH6sp9N2ycsoU=is3BVanH33CowD+sMNmWq=Z1MsPJX=HGYY+g@mail.gmail.com>
On Wed, 23 Nov 2011 09:17:43 +0100, Frans Klaver <fransklaver@gmail.com>
wrote:
> On Tue, Nov 22, 2011 at 10:31 AM, Frans Klaver <fransklaver@gmail.com>
> wrote:
>
>> If git is going to do some diagnostics on why the execvp returned
>> EACCES, it can still give a few hints. Most of the more likely options
>> are then ruled out.
>
> If there are no objections, I'm going to cook up a patch that
>
> - Keeps the current behavior (bail on EACCES)
> - Adds a more helpful diagnostic message somewhat like libexplain's,
> but more terse and if possible with slightly more domain knowledge
> - Takes into account the notes made following
> http://article.gmane.org/gmane.comp.version-control.git/171838
So here be some tests I intend to use (based on t0061.3):
run_command reports EACCES, file permissions:
cat hello-script >hello.sh &&
chmod -x hello.sh &&
test_must_fail test-run-command run-command ./hello.sh 2>err &&
grep "fatal: cannot exec.*hello.sh" err
run_command reports EACCES, search path permisions:
mkdir -p inaccessible &&
PATH=$(pwd)/inaccessible:$PATH &&
export PATH &&
cat hello-script >inaccessible/hello.sh &&
chmod 400 inaccessible &&
test_must_fail test-run-command run-command hello.sh 2>err &&
grep "fatal: cannot exec.*hello.sh" err &&
grep "incorrect PATH entry" err
run_command reports EACCES, interpreter fails:
cat incorrect-interpreter-script >hello.sh &&
chmod +x incorrect-interpreter-script &&
chmod -x someinterpreter &&
test_must_fail test-run-command run-command ./hello.sh 2>err &&
grep "fatal: cannot exec.*hello.sh" err &&
grep "cannot execute interpreter" err
Possibly getting (over)ambitious on the interpreter test, but hey, gotta
aim high.
If anybody has a test case that isn't covered, I'd be much obliged.
Frans
^ permalink raw reply
* Re: Proposal: create meaningful aliases for git reset's hard/soft/mixed
From: Philippe Vaucher @ 2011-11-23 23:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Christian Couder
In-Reply-To: <7vlir6brjw.fsf@alter.siamese.dyndns.org>
>> "I want to discard my changes" --> git reset --all HEAD^
>
> That is discarding your changes and also the last commit.
Yes, of course.
>> "I want to discard the last commit" --> git reset --index HEAD^
>
> I do not think this has a clear meaning. "discard the last commit but
> leave the contents in the working tree. I do not care a newly added files
> are forgotten by the index, I'll remember to re-add them if I need to" is
> what you are saying here, but the word "index" does not hint it. When
> used as an option name, "--index" means "this command usually works on or
> touches working tree but for this invocation please also affect the index";
> "please look at or affect _only_ the index" is usually spelled "--cached".
Well, it's certainly a bit more descriptive and easy to remember than
"--mixed". I understand it could confuse people because of the other
commands, but maybe something like "--index-only"?
> In any case, I think your proposal makes it even worse than the current
> state, and you should aim higher.
Why worse? I'd understand if you said it's doesn't improve it enough
for it to be worth the change tho.
Anyway, my proposal was to get a discussion going, and I'm all for
aiming higher if there's a way. What do you propose instead? You
seemed to imply we'd remove --soft and --merge, and make --keep as an
option for --hard but named differently, something like
--keep-changes. Maybe I didn't fully understand.
Mathieu even suggested that it'd have the behavior of --keep by
default, and that you have to add --force to get today's --hard
behavior, which sounds like a good idea to me (avoid destructive
behavior by default).
Philippe
^ permalink raw reply
* Re: Revisiting metadata storage
From: Jonathan Nieder @ 2011-11-24 1:10 UTC (permalink / raw)
To: Richard Hartmann; +Cc: Git List, Joey Hess, David Barr
In-Reply-To: <CAD77+gQB+0zJG62jrtPn_MwLLR7zgH=5gBtkvPxrKgiLPZsbsw@mail.gmail.com>
Richard Hartmann wrote:
> To make a long story short: Does anyone have a working solution,
> today?
Sure. etckeeper handles metadata such as owner and permissions
reasonably well.
^ permalink raw reply
* Re: [PATCH] Add test that checkout does not overwrite entries in .git/info/exclude
From: Nguyen Thai Ngoc Duy @ 2011-11-24 1:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Taylor Hedberg, Bertrand BENOIT, git
In-Reply-To: <7vy5v6bvy4.fsf@alter.siamese.dyndns.org>
On Thu, Nov 24, 2011 at 12:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> On Mon, Nov 21, 2011 at 10:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> In the medium term, I think one reasonable way forward solving the "TODO
>>> that used to be tracked but now untracked and ignored" issue is to
>>> introduce "info/exclude-override" that comes between command line and
>>> in-tree patterns. The info/exclude file is designed as the fallback
>>> definition to be used when all other sources are too lax, and comes near
>>> the precedence stack; the "TODO" situation however calls for an override
>>> that is stronger than the in-tree patterns.
>>
>> "info/precious" might be a better name
>
> The above is only about the precedence order and is not about introducing
> the new "precious" class at all.
The way I see how info/exclude-override helps keep (ignored but)
precious files is it un-ignores files that are ignored by info/exclude
and .gitignore. But if we go that way, then git-add will also
un-ignore precious files, allowing users to add them in-tree by
mistake.
>>> In the longer term, we should carefully determine if we need "precious" in
>>> the first place. The last time this was brought up there were people who
>>> argued they are OK with having to remove the ignored file by hand when
>>> checking out another branch (i.e. we switch the semantics of "ignored" so
>>> that they are "not tracked but all precious").
>>>
>>> I think it matters in two cases.
>>>
>>> (1) If you change an untracked "cruft" file on branch A into a directory
>>> with tracked files in it on another branch B. If you are on branch A,
>>> have that "cruft" file (perhaps it is a build product after running
>>> "make"), and try to checkout branch B, such an updated "git checkout"
>>> will start erroring out telling you that "cruft" will be lost.
>>>
>>> (2) If you have a directory on branch A, underneath of which there are
>>> untracked "cruft" files (e.g. think "build/" directory that is full
>>> of "*.o" files and ".gitignore" to mark object files as ignored but
>>> is otherwise empty), and another branch B that has the same path as a
>>> file. If you are on branch A, have "cruft" files in that directory,
>>> and try to checkout branch B, such an updated "git checkout" will
>>> start erroring out telling you that "cruft" will be lost.
>>
>> I think we should do this regardless precious being added or not.
>
> Because (see below)?
Because it may potentially lose user's changes. Assume file "A" is
tracked and also ignored. Users may make some changes in A, then move
HEAD away without touching worktree, now HEAD does not see "A" as
tracked any more. If we switch to a branch that has A tracked, git may
override A and wipe our user changes in that file. In short, if a file
is tracked (either in source or target in 2-way merge) then we ignore
excluded() result on that file.
>>> If people are OK with such a behaviour, we can do without "precious".
>>
>> What about git-clean to remove ignored but not precious files?
>
(git clean without -x snipped because I was talking about -x)
>
> "clean -x" is more subtle. It has been a way to say "Remove cruft the
> usual way, and in addition, remove the expendable build products, just
> like 'make clean' _should_ do, but I do not trust my Makefile". If we
> introduced "precious", it would be very clear what it should do---even
> with "-x" precious files should be kept. But if we don't and just try to
> get away by changing the semantics of "ignored", they will still need to
> be removed, so we won't really get the "precious".
>
> The conclusion from this is that it is a mistake to change the semantics
> of "ignored" from the current "untracked and expendable if needed" if the
> purpose of that change is to avoid introducing the new "precious" class.
>
> I don't care too much about it, as I do not use "git clean -x" myself ;-)
> but that wouldn't stop others from think about the issue and try to come
> up with a good solid design.
I don't use "git clean -x" myself either. This is just some input to
the discussion and true users are supposed to speak up. The point I
want to make is, if users really want the ability remove non-precious
ignored files, then we may need to add the new "precious" class that
(and you seem to reach the same conclusion).
--
Duy
^ permalink raw reply
* Re: [PATCH] Add test that checkout does not overwrite entries in .git/info/exclude
From: Junio C Hamano @ 2011-11-24 5:17 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Johannes Sixt, Taylor Hedberg, Bertrand BENOIT, git
In-Reply-To: <CACsJy8BYN-nu6a92dk-qpF5_jR_frKwg+DD1EThr2kMPGHV5kQ@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> ...
>>> I think we should do this regardless precious being added or not.
>>
>> Because (see below)?
>
> Because it may potentially lose user's changes. Assume file "A" is
> tracked and also ignored. Users may make some changes in A, then move
> HEAD away without touching worktree, now HEAD does not see "A" as
> tracked any more.
Huh? "A" is initially tracked and the user modifies it. Moving HEAD away
how? "git checkout" would refuse to check out a branch that has different
contents at "A", or is missing "A". So how can "now HEAD does not see 'A'
as tracked any more" happen?
^ permalink raw reply
* [PATCH] bulk-checkin: honor pack.packsizelimit
From: Junio C Hamano @ 2011-11-24 5:18 UTC (permalink / raw)
To: git
In-Reply-To: <1321600317-30546-1-git-send-email-gitster@pobox.com>
The bulk-checkin interface is designed to throw multiple blobs into a
single output packfile during the lifetime of a single process by
"plugging" the output. The direct streaming of the data to a packfile
however is primarily meant as a way to deal with large blobs better, and
it is possible that we end up with a single humongous packfile that is
awkward to handle.
Pay attention to the pack.packsizelimit configuration the same way as
the pack-object does, and make sure we close a packfile and switch to a
new one before busting the size limit.
We allow the limit to be busted if a single object is too large to be
contained in a pack that is smaller than the limit on its own, as there is
no way to store such an object otherwise; the same is already done in
pack-objects.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/pack-objects.c | 6 +-----
bulk-checkin.c | 35 ++++++++++++++++++++++++++++++-----
cache.h | 1 +
config.c | 4 ++++
environment.c | 1 +
5 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index b458b6d..dde913e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -76,7 +76,7 @@ static struct pack_idx_option pack_idx_opts;
static const char *base_name;
static int progress = 1;
static int window = 10;
-static unsigned long pack_size_limit, pack_size_limit_cfg;
+static unsigned long pack_size_limit;
static int depth = 50;
static int delta_search_threads;
static int pack_to_stdout;
@@ -2009,10 +2009,6 @@ static int git_pack_config(const char *k, const char *v, void *cb)
pack_idx_opts.version);
return 0;
}
- if (!strcmp(k, "pack.packsizelimit")) {
- pack_size_limit_cfg = git_config_ulong(k, v);
- return 0;
- }
return git_default_config(k, v, cb);
}
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 60178ef..2adc67b 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -73,10 +73,13 @@ static int already_written(struct bulk_checkin_state *state, unsigned char sha1[
return 0;
}
-static void deflate_to_pack(struct bulk_checkin_state *state,
- unsigned char sha1[],
- int fd, size_t size, enum object_type type,
- const char *path, unsigned flags)
+#define DEFLATE_TO_PACK_OK 0
+#define DEFLATE_TO_PACK_TOOBIG 1
+
+static int deflate_to_pack(struct bulk_checkin_state *state,
+ unsigned char sha1[],
+ int fd, size_t size, enum object_type type,
+ const char *path, unsigned flags)
{
unsigned char obuf[16384];
unsigned hdrlen;
@@ -149,6 +152,13 @@ static void deflate_to_pack(struct bulk_checkin_state *state,
sha1file_truncate(state->f, &checkpoint);
state->offset = checkpoint.offset;
free(idx);
+ } else if (state->nr_written &&
+ pack_size_limit_cfg &&
+ pack_size_limit_cfg < state->offset) {
+ sha1file_truncate(state->f, &checkpoint);
+ state->offset = checkpoint.offset;
+ free(idx);
+ return DEFLATE_TO_PACK_TOOBIG;
} else {
hashcpy(idx->sha1, sha1);
ALLOC_GROW(state->written,
@@ -156,12 +166,17 @@ static void deflate_to_pack(struct bulk_checkin_state *state,
state->written[state->nr_written++] = idx;
}
}
+ return DEFLATE_TO_PACK_OK;
}
int index_bulk_checkin(unsigned char *sha1,
int fd, size_t size, enum object_type type,
const char *path, unsigned flags)
{
+ off_t seekback;
+ int status;
+
+again:
if (!state.f && (flags & HASH_WRITE_OBJECT)) {
state.f = create_tmp_packfile(&state.pack_tmp_name);
reset_pack_idx_option(&state.pack_idx_opts);
@@ -171,7 +186,17 @@ int index_bulk_checkin(unsigned char *sha1,
die_errno("unable to write pack header");
}
- deflate_to_pack(&state, sha1, fd, size, type, path, flags);
+ seekback = lseek(fd, 0, SEEK_CUR);
+ if (seekback == (off_t) -1)
+ return error("cannot seek");
+ status = deflate_to_pack(&state, sha1, fd, size, type, path, flags);
+ if (status == DEFLATE_TO_PACK_TOOBIG) {
+ finish_bulk_checkin(&state);
+ if (lseek(fd, seekback, SEEK_SET) == (off_t) -1)
+ return error("cannot seek back");
+ goto again;
+ }
+
if (!state.plugged)
finish_bulk_checkin(&state);
return 0;
diff --git a/cache.h b/cache.h
index 2e6ad36..b158d3e 100644
--- a/cache.h
+++ b/cache.h
@@ -598,6 +598,7 @@ extern size_t packed_git_window_size;
extern size_t packed_git_limit;
extern size_t delta_base_cache_limit;
extern unsigned long big_file_threshold;
+extern unsigned long pack_size_limit_cfg;
extern int read_replace_refs;
extern int fsync_object_files;
extern int core_preload_index;
diff --git a/config.c b/config.c
index edf9914..c736802 100644
--- a/config.c
+++ b/config.c
@@ -797,6 +797,10 @@ int git_default_config(const char *var, const char *value, void *dummy)
return 0;
}
+ if (!strcmp(var, "pack.packsizelimit")) {
+ pack_size_limit_cfg = git_config_ulong(var, value);
+ return 0;
+ }
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/environment.c b/environment.c
index 0bee6a7..31e4284 100644
--- a/environment.c
+++ b/environment.c
@@ -60,6 +60,7 @@ char *notes_ref_name;
int grafts_replace_parents = 1;
int core_apply_sparse_checkout;
struct startup_info *startup_info;
+unsigned long pack_size_limit_cfg;
/* Parallel index stat data preload? */
int core_preload_index = 0;
--
1.7.8.rc3.208.g1edbd
^ permalink raw reply related
* Re: [PATCH] Add test that checkout does not overwrite entries in .git/info/exclude
From: Nguyen Thai Ngoc Duy @ 2011-11-24 5:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Taylor Hedberg, Bertrand BENOIT, git
In-Reply-To: <7vlir69k0s.fsf@alter.siamese.dyndns.org>
On Thu, Nov 24, 2011 at 12:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> ...
>>>> I think we should do this regardless precious being added or not.
>>>
>>> Because (see below)?
>>
>> Because it may potentially lose user's changes. Assume file "A" is
>> tracked and also ignored. Users may make some changes in A, then move
>> HEAD away without touching worktree, now HEAD does not see "A" as
>> tracked any more.
>
> Huh? "A" is initially tracked and the user modifies it. Moving HEAD away
> how? "git checkout" would refuse to check out a branch that has different
> contents at "A", or is missing "A". So how can "now HEAD does not see 'A'
> as tracked any more" happen?
git reset <somewhere else>
--
Duy
^ permalink raw reply
* Re: git-bisect working only from toplevel dir
From: Peter Baumann @ 2011-11-24 7:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Adam Borowski, git
In-Reply-To: <7vzkfma7q9.fsf@alter.siamese.dyndns.org>
On Wed, Nov 23, 2011 at 12:45:18PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Wed, Nov 23, 2011 at 11:09:29AM -0800, Junio C Hamano wrote:
> >
> >> As to the approach, I suspect that it would be far better if it made
> >> workable with cd_to_toplevel at the beginning, instead of saying
> >> SUBDIRECTORY_OK.
> >>
> >> After all, the current directory may disappear during the course of
> >> bisection, upon checking out a revision that did not have the directory
> >> you started your bisection from.
> >
> > But from what directory would you expect:
> >
> > git bisect run make
>
> My usual way to enlighten somebody is by forcing him/her to think the
> consequences, but because you did the thinking for the OP in this thread
> instead, it didn't work. Makes me somewhat sad ;-<.
>
> > If we cd_to_toplevel, we can remember the prefix that we started from
> > and cd to it before running the user's command, but there is no
> > guarantee that it actually exists. Maybe that commit should be
> > considered indeterminate then?
>
> Yeah that sounds like a reasonable thing to do.
>
> > I dunno. I haven't thought that hard about it. But I don't think it's
> > quite as simple as just telling bisect it's OK to run from a subdir.
>
> Absolutely. Saying SUBDIRECTORY_OK without thinking about the consequence
> through is a good discussion starter but is not a good patch.
>
> Also didn't we make bisect workable in a bare repository recently? So the
> start-up sequence has to be something more elaborate like...
>
> . git-sh-setup
> if we are in a bare repository
> then
> : we are happy...nothing funky needs to be done
> elif we are not in a working tree
> barf
> elif we are not at the top
> prefix=$(git rev-parse --show-prefix)
> cd_to_toplevel
> fi
>
> and then inside bisect_next() you would check if $prefix exists, and go
> there to run bisect--helper (or fail to go there and say "cannot test").
>
But is the "cannot test" aka exit(127) the best we can do in this case?
I think having a failing make, because someone has checked in code which doesn't
even compile, is something totally different than having no Makefile at all,
because the directory doesn't even exist. To me, this seems more like an error
in the run script to not handle all the cases of the (dis)appearing directory.
On the other hand we don't waste much time trying to test such an "untestable"
commit, because this check will be fast because no compiling is involved.
The only time wasted will bethe build time for the "usable" commits and the
time the user needs to figure out why the heck some(/most?) of his commits
are "untestable".
^ permalink raw reply
* Re: git fetch overwriting local tags
From: Uwe Kleine-König @ 2011-11-24 7:07 UTC (permalink / raw)
To: Jeff King; +Cc: git, John Kacur
In-Reply-To: <20111123221658.GA22313@sigill.intra.peff.net>
Hi Jeff,
On Wed, Nov 23, 2011 at 05:16:58PM -0500, Jeff King wrote:
> On Wed, Nov 23, 2011 at 10:08:21AM +0100, Uwe Kleine-König wrote:
>
> > John and I wondered about git fetch overwriting local tags. I was sure
> > enough to claim that git fetch won't overwrite local tags with remote
> > tags having the same name. But after John pointed me to
> >
> > http://www.pythian.com/news/9067/on-the-perils-of-importing-remote-tags-in-git/
> >
> > I tested that (using Debian's 1.7.7.3) and really, git does overwrite
> > local tags.
> >
> > Here is my test script:
> > [...]
> > git fetch --tags ../a
> > [...]
> > Is this intended?
>
> Sort of.
>
> By default, "git fetch" will "auto-follow" tags; if you fetch a commit
> which is pointed to by a tag, then git will fetch that tag, too. So
> generally, you shouldn't need to specify "--tags" at all, because you
> will already be getting the relevant tags.
Hmm, if I do:
mkdir a
cd a
echo some content > some_file
git init
git add some_file
git commit -m 'some commit log'
git tag some_tag
cd ..
mkdir b
cd b
echo some different content > another_file
git init
git add another_file
git commit -m 'another commit log'
git fetch ../a
I don't get the tag. That's why I added --tags. I guess that's because
some_tag is a lightweight tag. Hmm, but even if I change the command to
create the tag to
git tag -a -m 'tag desc' some_tag
I don't get it without --tags?!
> The "--tags" option, however, is a short-hand for saying "fetch all of
> the tags", and is equivalent to providing the refspec:
>
> git fetch ../a refs/tags/*:refs/tags/*
>
> Which of course will update your local tags with similarly-named ones
> from the remote. So in that sense, there is no bug, and it is working
> as intended; the problem is that the author's intent was not the same as
> your intent. :)
>
> I'm not sure why you're using "--tags" in the first place. That might
> help us figure out if there's another way to do what you want that is
> safer.
>
> That being said, it would be nice if "--tags" wasn't so surprising.
> Three things that I think could help are:
>
> 1. We usually require a "+" on the refspec (or "--force") to update
> non-fast-forward branches. But there is no such safety on tags
> (which generally shouldn't be updated at all). Should we at least
> be enforcing the same fast-forward rules on tag fetches (or even
> something more strict, like forbidding tag update at all unless
> forced)?
That sounds fine for me.
> 2. We don't keep a reflog on tags. Generally there's no point. But
> it wouldn't be very expensive (since they don't usually change),
> and could provide a safety mechanism here.
I prefer 1, but that would be better than the current situation at
least.
> 3. Keeping tags from remotes in separate namespaces, but collating
> them at lookup time. This has been discussed, and I think is
> generally a fine idea, but nobody has moved forward with code.
That's something that John said in our discussion, too. That's the
suggestion I like the most.
Best regards and thanks for your time,
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] make git-svn resilient to log.abbrevcommit = true
From: Shahid Alam @ 2011-11-24 7:16 UTC (permalink / raw)
To: git, gitster
Add --no-abbrev-commit arg to working_head_Info()'s invocation
of git log.
---
git-svn.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 351e743..da4c240 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1803,7 +1803,7 @@ sub cmt_sha2rev_batch {
sub working_head_info {
my ($head, $refs) = @_;
my @args = qw/log --no-color --no-decorate --first-parent
- --pretty=medium/;
+ --no-abbrev-commit --pretty=medium/;
my ($fh, $ctx) = command_output_pipe(@args, $head);
my $hash;
my %max;
--
1.7.7.2
^ permalink raw reply related
* Let Us Work Together.
From: Desmond William @ 2011-11-24 10:23 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 130 bytes --]
I bring to your notice vital business that will yield huge gains.... It will be yielding if you read the attached file and reply
[-- Attachment #2: contract.awrd.rtf --]
[-- Type: application/octet-stream, Size: 2117 bytes --]
Contract Awarding committee Abidjan Cote D` Ivoire
Greetings,
Please allow me to intimate you with a business transaction that would benefit both of us. My name is Dr Desmond Williams, I am the director of the project and engineering division of SODECI (Soci?t? de distribution d'eau de C?te d'Ivoire). I will want you to assist me in the claim and transfer of $25 Million United States of American Dollars.
This amount was realized from over-invoiced contract amount awarded by us in 2007 to expatriate companies. The Contract's Amount was deliberately overestimated to read more than the actual cost of the project thus leaving a surplus of $25 Million United States of American Dollars. However, as a head staff of SODECI, I cannot claim this money without a foreign partner who will act as the contractor, entitled to receive the funds as his outstanding Contract payment. As I want to use umbrella of your company in the name of the original contractor to claim this fund, the original contractor has received his complete payment leaving over invoice but as political war continue in C?te d'Ivoire, which delays this payment.
France and UN recognized Elected President of Cote d`Ivoire Alassane Ouattara appointed prime Minster formal rebel leader secretary Mr. SOLO GUILLAUME who has decided to pay off all outstanding contract debts to redeem back the image of the country. Following the suffered by companies and contractor here is Abidjan C?te d'Ivoire while waiting their payments, available now via Cote D`Ivoire Government accredited paying banks City Bank Group Via African Development Bank. Sequel to this I need your Company letter headed paper with address only for us to proceed.
Please show me your willing to assist me in this transaction by getting back to me so I can give you further details about this transaction. I am willing to give you 25% of the total amount after the transfer, the rest will be held and released to me when I get to your country. I`m look forward to hear from you.
Regards
Dr Desmond Williams
Telephone (+22504202926)
deswilliamsp1@yahoo.co.jp
^ permalink raw reply
* [PATCH 0/13] credential helpers, take two
From: Jeff King @ 2011-11-24 10:58 UTC (permalink / raw)
To: git
Here's a revised version of the http-auth / credential-helper series.
It has the same basic premise as the first series (git contacts external
programs to access secure storage, thus enabling secure password
caching), but there are a number of tweaks in the implementation.
The main departures are:
1. Helpers now _only_ act as storage; they never ask for credentials
directly from the user. This makes writing them much simpler.
It also means that "askpass" remains the only way to get input from
the user. However, I've left room in the helper protocol to add an
"ask" action. If people really want something like a classy
username/password dialog from the helpers, it will be easy to add.
2. The helper protocol now happens completely over pipes. In the first
round, we gave information to helpers on the command line. That was
OK, since we never gave them a password; they only gave them to us
(and cached them if they wanted to). But now that git is asking for
the password itself, it has to send the password to the helper to
store. And that definitely shouldn't go on the command line.
The parsing load on the helper is therefore a little higher.
However, it's still really quite easy.
3. The old "unique" token has been broken into components. That means
less parsing for most helpers, which wanted the broken-down fields.
Helpers that want a single token can pretty easily reassemble.
4. I dropped the "description" field. I noticed that all of the
components of a credential context are actually parts of a URL. So
we can just show the URL (or a subset thereof) to the user in the
prompt. See the discussion in patches 05 and 08.
5. Config handling happens at the right place (before helpers) now.
If you want an overview without reading the patches too carefully, I
recommend reading the documentation added in patches 03 and 09, which
contain the API and end-user documentation respectively.
Helper writers may want to look at t0303 added in patch 13; it's an
adaptation of the test script I posted earlier for testing new external
helpers.
[01/13]: test-lib: add test_config_global variant
[02/13]: t5550: fix typo
[03/13]: introduce credentials API
[04/13]: credential: add function for parsing url components
[05/13]: http: use credential API to get passwords
[06/13]: credential: apply helper config
[07/13]: credential: add credential.*.username
[08/13]: credential: make relevance of http path configurable
[09/13]: docs: end-user documentation for the credential subsystem
[10/13]: credentials: add "cache" helper
[11/13]: strbuf: add strbuf_add*_urlencode
[12/13]: credentials: add "store" helper
[13/13]: t: add test harness for external credential helpers
I've been running with this for a few days, so I think the most horrible
bugs are shaken out. But I had to rewrite almost all of the code from
the first series, so I wouldn't be surprised if there are some new bugs
lurking in there. If you are reviewing, please read from scratch and
don't assume that something that worked in the first series is still
working. :)
-Peff
^ permalink raw reply
* [PATCH 01/13] test-lib: add test_config_global variant
From: Jeff King @ 2011-11-24 10:58 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
The point of test_config is to simultaneously set a config
variable and register its cleanup handler, like:
test_config core.foo bar
However, it stupidly assumes that $1 contained the name of
the variable, which means it won't work for:
test_config --global core.foo bar
We could try to parse the command-line ourselves and figure
out which parts need to be fed to test_unconfig. But since
this is likely the most common variant, it's much simpler
and less error-prone to simply add a new function.
Signed-off-by: Jeff King <peff@peff.net>
---
t/test-lib.sh | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..160479b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -379,6 +379,11 @@ test_config () {
git config "$@"
}
+test_config_global () {
+ test_when_finished "test_unconfig --global '$1'" &&
+ git config --global "$@"
+}
+
# Use test_set_prereq to tell that a particular prerequisite is available.
# The prerequisite can later be checked for in two ways:
#
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
* [PATCH 02/13] t5550: fix typo
From: Jeff King @ 2011-11-24 10:59 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
This didn't have an impact, because it was just setting up
an "expect" file that happened to be identical to the one in
the test before it.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t5550-http-fetch.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh
index 311a33c..3d6e871 100755
--- a/t/t5550-http-fetch.sh
+++ b/t/t5550-http-fetch.sh
@@ -66,7 +66,7 @@ test_expect_success 'cloning password-protected repository can fail' '
test_expect_success 'http auth can use user/pass in URL' '
>askpass-query &&
- echo wrong >askpass-reponse &&
+ echo wrong >askpass-response &&
git clone "$HTTPD_URL_USER_PASS/auth/repo.git" clone-auth-none &&
test_cmp askpass-expect-none askpass-query
'
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
* [PATCH 03/13] introduce credentials API
From: Jeff King @ 2011-11-24 11:01 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
There are a few places in git that need to get a username
and password credential from the user; the most notable one
is HTTP authentication for smart-http pushing.
Right now the only choices for providing credentials are to
put them plaintext into your ~/.netrc, or to have git prompt
you (either on the terminal or via an askpass program). The
former is not very secure, and the latter is not very
convenient.
Unfortunately, there is no "always best" solution for
password management. The details will depend on the tradeoff
you want between security and convenience, as well as how
git can integrate with other security systems (e.g., many
operating systems provide a keychain or password wallet for
single sign-on).
This patch provides an abstract notion of credentials as a
data item, and provides three basic operations:
- fill (i.e., acquire from external storage or from the
user)
- approve (mark a credential as "working" for further
storage)
- reject (mark a credential as "not working", so it can
be removed from storage)
These operations can be backed by external helper processes
that interact with system- or user-specific secure storage.
Signed-off-by: Jeff King <peff@peff.net>
---
.gitignore | 1 +
Documentation/technical/api-credentials.txt | 148 ++++++++++++++++
Makefile | 3 +
credential.c | 242 +++++++++++++++++++++++++++
credential.h | 28 +++
t/lib-credential.sh | 33 ++++
t/t0300-credentials.sh | 195 +++++++++++++++++++++
test-credential.c | 38 ++++
8 files changed, 688 insertions(+), 0 deletions(-)
create mode 100644 Documentation/technical/api-credentials.txt
create mode 100644 credential.c
create mode 100644 credential.h
create mode 100755 t/lib-credential.sh
create mode 100755 t/t0300-credentials.sh
create mode 100644 test-credential.c
diff --git a/.gitignore b/.gitignore
index 8572c8c..7d2fefc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -167,6 +167,7 @@
/gitweb/static/gitweb.js
/gitweb/static/gitweb.min.*
/test-chmtime
+/test-credential
/test-ctype
/test-date
/test-delta
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
new file mode 100644
index 0000000..3061077
--- /dev/null
+++ b/Documentation/technical/api-credentials.txt
@@ -0,0 +1,148 @@
+credentials API
+===============
+
+The credentials API provides an abstracted way of gathering username and
+password credentials from the user (even though credentials in the wider
+world can take many forms, in this document the word "credential" always
+refers to a username and password pair).
+
+Data Structures
+---------------
+
+`struct credential`::
+
+ This struct represents a single username/password combination
+ along with any associated context. All string fields should be
+ heap-allocated (or NULL if they are not known or not applicable).
+ The meaning of the individual context fields is the same as
+ their counterparts in the helper protocol; see the section below
+ for a description of each field.
++
+The `helpers` member of the struct is a `string_list` of helpers. Each
+string specifies an external helper which will be run, in order, to
+either acquire or store credentials. See the section on credential
+storage helpers below.
++
+This struct should always be initialized with `CREDENTIAL_INIT` or
+`credential_init`.
+
+
+Functions
+---------
+
+`credential_init`::
+
+ Initialize a credential structure, setting all fields to empty.
+
+`credential_clear`::
+
+ Free any resources associated with the credential structure,
+ returning it to a pristine initialized state.
+
+`credential_fill`::
+
+ Attempt to fill the username and password fields of the passed
+ credential struct, first consulting storage helpers, then asking
+ the user. Guarantees that the username and password fields will
+ be filled afterwards (or die() will be called).
+
+`credential_reject`::
+
+ Inform the credential subsystem that the provided credentials
+ have been rejected. This will notify any storage helpers of the
+ rejection (which allows them to, for example, purge the invalid
+ credentials from storage), and then clear the username and
+ password fields in `struct credential`. It can then be
+ `credential_fill`-ed again.
+
+`credential_approve`::
+
+ Inform the credential subsystem that the provided credentials
+ were successfully used for authentication. This will notify any
+ storage helpers of the approval, so that they can store the
+ result to be used again.
+
+
+Credential Storage Helpers
+--------------------------
+
+Credential storage helpers are programs executed by git to fetch or save
+credentials from and to long-term storage (where "long-term" is simply
+longer than a single git process; e.g., credentials may be stored
+in-memory for a few minutes, or indefinitely on disk).
+
+Helper scripts should generally be found in the PATH, and have names of
+the form "git-credential-$HELPER". When the helper string "$HELPER" is
+passed to credential functions, they will run "git-credential-$HELPER"
+via the shell. If the first word of $HELPER contains non-alphanumeric
+characters, then $HELPER is executed as a shell command. This makes it
+possible to specify individual scripts by their full path (e.g.,
+`/path/to/helper`) or even shell snippets (`f() { do_whatever; }; f`).
+
+When a helper is executed, it will have one "operation" argument
+appended to its command line, which is one of:
+
+`get`::
+
+ Return a matching credential, if any exists.
+
+`store`::
+
+ Store the credential, if applicable to the helper.
+
+`erase`::
+
+ Remove a matching credential, if any, from the helper's storage.
+
+The details of the credential will be provided on the helper's stdin
+stream. The credential is split into a set of named attributes.
+Attributes are provided to the helper, one per line. Each attribute is
+specified by a key-value pair, separated by an `=` (equals) sign,
+followed by a newline. The key may contain any bytes except `=` or
+newline. The value may contain any bytes except a newline. In both
+cases, all bytes are treated as-is (i.e., there is no quoting, and one
+cannot transmit a value with newline in it). The list of attributes is
+terminated by a blank line or end-of-file.
+
+Git will send the following attributes (but may not send all of
+them for a given credential; for example, a `host` attribute makes no
+sense when dealing with a non-network protocol):
+
+`protocol`::
+
+ The protocol over which the credential will be used (e.g.,
+ `https`).
+
+`host`::
+
+ The remote hostname for a network credential.
+
+`path`::
+
+ The path with which the credential will be used. E.g., for
+ accessing a remote https repository, this will be the
+ repository's path on the server.
+
+`username`::
+
+ The credential's username, if we already have one (e.g., from a
+ URL, from the user, or from a previously run helper).
+
+`password`::
+
+ The credential's password, if we are asking it to be stored.
+
+For a `get` operation, the helper should produce a list of attributes
+on stdout in the same format. A helper is free to produce a subset, or
+even no values at all if it has nothing useful to provide. Any provided
+attributes will overwrite those already known about by git.
+
+For a `store` or `erase` operation, the helper's output is ignored.
+If it fails to perform the requested operation, it may complain to
+stderr to inform the user. If it does not support the requested
+operation (e.g., a read-only store), it should silently ignore the
+request.
+
+If a helper receives any other operation, it should silently ignore the
+request. This leaves room for future operations to be added (older
+helpers will just ignore the new requests).
diff --git a/Makefile b/Makefile
index b1c80a6..5ca363b 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ PROGRAM_OBJS += sh-i18n--envsubst.o
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-credential
TEST_PROGRAMS_NEED_X += test-ctype
TEST_PROGRAMS_NEED_X += test-date
TEST_PROGRAMS_NEED_X += test-delta
@@ -525,6 +526,7 @@ LIB_H += compat/win32/poll.h
LIB_H += compat/win32/dirent.h
LIB_H += connected.h
LIB_H += convert.h
+LIB_H += credential.h
LIB_H += csum-file.h
LIB_H += decorate.h
LIB_H += delta.h
@@ -609,6 +611,7 @@ LIB_OBJS += connect.o
LIB_OBJS += connected.o
LIB_OBJS += convert.o
LIB_OBJS += copy.o
+LIB_OBJS += credential.o
LIB_OBJS += csum-file.o
LIB_OBJS += ctype.o
LIB_OBJS += date.o
diff --git a/credential.c b/credential.c
new file mode 100644
index 0000000..d50a611
--- /dev/null
+++ b/credential.c
@@ -0,0 +1,242 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+#include "run-command.h"
+
+void credential_init(struct credential *c)
+{
+ memset(c, 0, sizeof(*c));
+ c->helpers.strdup_strings = 1;
+}
+
+void credential_clear(struct credential *c)
+{
+ free(c->protocol);
+ free(c->host);
+ free(c->path);
+ free(c->username);
+ free(c->password);
+ string_list_clear(&c->helpers, 0);
+
+ credential_init(c);
+}
+
+static void credential_describe(struct credential *c, struct strbuf *out)
+{
+ if (!c->protocol)
+ return;
+ strbuf_addf(out, "%s://", c->protocol);
+ if (c->username && *c->username)
+ strbuf_addf(out, "%s@", c->username);
+ if (c->host)
+ strbuf_addstr(out, c->host);
+ if (c->path)
+ strbuf_addf(out, "/%s", c->path);
+}
+
+static char *credential_ask_one(const char *what, struct credential *c)
+{
+ struct strbuf desc = STRBUF_INIT;
+ struct strbuf prompt = STRBUF_INIT;
+ char *r;
+
+ credential_describe(c, &desc);
+ if (desc.len)
+ strbuf_addf(&prompt, "%s for '%s': ", what, desc.buf);
+ else
+ strbuf_addf(&prompt, "%s: ", what);
+
+ /* FIXME: for usernames, we should do something less magical that
+ * actually echoes the characters. However, we need to read from
+ * /dev/tty and not stdio, which is not portable (but getpass will do
+ * it for us). http.c uses the same workaround. */
+ r = git_getpass(prompt.buf);
+
+ strbuf_release(&desc);
+ strbuf_release(&prompt);
+ return xstrdup(r);
+}
+
+static void credential_getpass(struct credential *c)
+{
+ if (!c->username)
+ c->username = credential_ask_one("Username", c);
+ if (!c->password)
+ c->password = credential_ask_one("Password", c);
+}
+
+int credential_read(struct credential *c, FILE *fp)
+{
+ struct strbuf line = STRBUF_INIT;
+
+ while (strbuf_getline(&line, fp, '\n') != EOF) {
+ char *key = line.buf;
+ char *value = strchr(key, '=');
+
+ if (!line.len)
+ break;
+
+ if (!value) {
+ warning("invalid credential line: %s", key);
+ strbuf_release(&line);
+ return -1;
+ }
+ *value++ = '\0';
+
+ if (!strcmp(key, "username")) {
+ free(c->username);
+ c->username = xstrdup(value);
+ }
+ else if (!strcmp(key, "password")) {
+ free(c->password);
+ c->password = xstrdup(value);
+ }
+ else if (!strcmp(key, "protocol")) {
+ free(c->protocol);
+ c->protocol = xstrdup(value);
+ }
+ else if (!strcmp(key, "host")) {
+ free(c->host);
+ c->host = xstrdup(value);
+ }
+ else if (!strcmp(key, "path")) {
+ free(c->path);
+ c->path = xstrdup(value);
+ }
+ /* ignore other lines; we don't know what they mean, but
+ * this future-proofs us when later versions of git do
+ * learn new lines, and the helpers are updated to match */
+ }
+
+ strbuf_release(&line);
+ return 0;
+}
+
+static void credential_write_item(FILE *fp, const char *key, const char *value)
+{
+ if (!value)
+ return;
+ fprintf(fp, "%s=%s\n", key, value);
+}
+
+static void credential_write(const struct credential *c, FILE *fp)
+{
+ credential_write_item(fp, "protocol", c->protocol);
+ credential_write_item(fp, "host", c->host);
+ credential_write_item(fp, "path", c->path);
+ credential_write_item(fp, "username", c->username);
+ credential_write_item(fp, "password", c->password);
+}
+
+static int first_word_is_alnum(const char *s)
+{
+ for (; *s && *s != ' '; s++)
+ if (!isalnum(*s))
+ return 0;
+ return 1;
+}
+
+static int run_credential_helper(struct credential *c,
+ const char *cmd,
+ int want_output)
+{
+ struct child_process helper;
+ const char *argv[] = { NULL, NULL };
+ FILE *fp;
+
+ memset(&helper, 0, sizeof(helper));
+ argv[0] = cmd;
+ helper.argv = argv;
+ helper.use_shell = 1;
+ helper.in = -1;
+ if (want_output)
+ helper.out = -1;
+ else
+ helper.no_stdout = 1;
+
+ if (start_command(&helper) < 0)
+ return -1;
+
+ fp = xfdopen(helper.in, "w");
+ credential_write(c, fp);
+ fclose(fp);
+
+ if (want_output) {
+ int r;
+ fp = xfdopen(helper.out, "r");
+ r = credential_read(c, fp);
+ fclose(fp);
+ if (r < 0) {
+ finish_command(&helper);
+ return -1;
+ }
+ }
+
+ if (finish_command(&helper))
+ return -1;
+ return 0;
+}
+
+static int credential_do(struct credential *c, const char *method,
+ const char *operation)
+{
+ struct strbuf cmd = STRBUF_INIT;
+ int r;
+
+ if (first_word_is_alnum(method))
+ strbuf_addf(&cmd, "git credential-%s", method);
+ else
+ strbuf_addstr(&cmd, method);
+ strbuf_addf(&cmd, " %s", operation);
+
+ r = run_credential_helper(c, cmd.buf, !strcmp(operation, "get"));
+
+ strbuf_release(&cmd);
+ return r;
+}
+
+void credential_fill(struct credential *c)
+{
+ int i;
+
+ if (c->username && c->password)
+ return;
+
+ for (i = 0; i < c->helpers.nr; i++) {
+ credential_do(c, c->helpers.items[i].string, "get");
+ if (c->username && c->password)
+ return;
+ }
+
+ credential_getpass(c);
+ if (!c->username && !c->password)
+ die("unable to get password from user");
+}
+
+void credential_approve(struct credential *c)
+{
+ int i;
+
+ if (c->approved)
+ return;
+ if (!c->username || !c->password)
+ return;
+
+ for (i = 0; i < c->helpers.nr; i++)
+ credential_do(c, c->helpers.items[i].string, "store");
+ c->approved = 1;
+}
+
+void credential_reject(struct credential *c)
+{
+ int i;
+
+ for (i = 0; i < c->helpers.nr; i++)
+ credential_do(c, c->helpers.items[i].string, "erase");
+
+ free(c->username);
+ c->username = NULL;
+ free(c->password);
+ c->password = NULL;
+ c->approved = 0;
+}
diff --git a/credential.h b/credential.h
new file mode 100644
index 0000000..2ea7d49
--- /dev/null
+++ b/credential.h
@@ -0,0 +1,28 @@
+#ifndef CREDENTIAL_H
+#define CREDENTIAL_H
+
+#include "string-list.h"
+
+struct credential {
+ struct string_list helpers;
+ unsigned approved:1;
+
+ char *username;
+ char *password;
+ char *protocol;
+ char *host;
+ char *path;
+};
+
+#define CREDENTIAL_INIT { STRING_LIST_INIT_DUP }
+
+void credential_init(struct credential *);
+void credential_clear(struct credential *);
+
+void credential_fill(struct credential *);
+void credential_approve(struct credential *);
+void credential_reject(struct credential *);
+
+int credential_read(struct credential *, FILE *);
+
+#endif /* CREDENTIAL_H */
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
new file mode 100755
index 0000000..54ae1f4
--- /dev/null
+++ b/t/lib-credential.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+# Try a set of credential helpers; the expected stdin,
+# stdout and stderr should be provided on stdin,
+# separated by "--".
+check() {
+ read_chunk >stdin &&
+ read_chunk >expect-stdout &&
+ read_chunk >expect-stderr &&
+ test-credential "$@" <stdin >stdout 2>stderr &&
+ test_cmp expect-stdout stdout &&
+ test_cmp expect-stderr stderr
+}
+
+read_chunk() {
+ while read line; do
+ case "$line" in
+ --) break ;;
+ *) echo "$line" ;;
+ esac
+ done
+}
+
+
+cat >askpass <<\EOF
+#!/bin/sh
+echo >&2 askpass: $*
+what=`echo $1 | cut -d" " -f1 | tr A-Z a-z | tr -cd a-z`
+echo "askpass-$what"
+EOF
+chmod +x askpass
+GIT_ASKPASS="$PWD/askpass"
+export GIT_ASKPASS
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
new file mode 100755
index 0000000..81a455f
--- /dev/null
+++ b/t/t0300-credentials.sh
@@ -0,0 +1,195 @@
+#!/bin/sh
+
+test_description='basic credential helper tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-credential.sh
+
+test_expect_success 'setup helper scripts' '
+ cat >dump <<-\EOF &&
+ whoami=`echo $0 | sed s/.*git-credential-//`
+ echo >&2 "$whoami: $*"
+ while IFS== read key value; do
+ echo >&2 "$whoami: $key=$value"
+ eval "$key=$value"
+ done
+ EOF
+
+ cat >git-credential-useless <<-\EOF &&
+ #!/bin/sh
+ . ./dump
+ exit 0
+ EOF
+ chmod +x git-credential-useless &&
+
+ cat >git-credential-verbatim <<-\EOF &&
+ #!/bin/sh
+ user=$1; shift
+ pass=$1; shift
+ . ./dump
+ test -z "$user" || echo username=$user
+ test -z "$pass" || echo password=$pass
+ EOF
+ chmod +x git-credential-verbatim &&
+
+ PATH="$PWD:$PATH"
+'
+
+test_expect_success 'credential_fill invokes helper' '
+ check fill "verbatim foo bar" <<-\EOF
+ --
+ username=foo
+ password=bar
+ --
+ verbatim: get
+ EOF
+'
+
+test_expect_success 'credential_fill invokes multiple helpers' '
+ check fill useless "verbatim foo bar" <<-\EOF
+ --
+ username=foo
+ password=bar
+ --
+ useless: get
+ verbatim: get
+ EOF
+'
+
+test_expect_success 'credential_fill stops when we get a full response' '
+ check fill "verbatim one two" "verbatim three four" <<-\EOF
+ --
+ username=one
+ password=two
+ --
+ verbatim: get
+ EOF
+'
+
+test_expect_success 'credential_fill continues through partial response' '
+ check fill "verbatim one \"\"" "verbatim two three" <<-\EOF
+ --
+ username=two
+ password=three
+ --
+ verbatim: get
+ verbatim: get
+ verbatim: username=one
+ EOF
+'
+
+test_expect_success 'credential_fill passes along metadata' '
+ check fill "verbatim one two" <<-\EOF
+ protocol=ftp
+ host=example.com
+ path=foo.git
+ --
+ username=one
+ password=two
+ --
+ verbatim: get
+ verbatim: protocol=ftp
+ verbatim: host=example.com
+ verbatim: path=foo.git
+ EOF
+'
+
+test_expect_success 'credential_approve calls all helpers' '
+ check approve useless "verbatim one two" <<-\EOF
+ username=foo
+ password=bar
+ --
+ --
+ useless: store
+ useless: username=foo
+ useless: password=bar
+ verbatim: store
+ verbatim: username=foo
+ verbatim: password=bar
+ EOF
+'
+
+test_expect_success 'do not bother storing password-less credential' '
+ check approve useless <<-\EOF
+ username=foo
+ --
+ --
+ EOF
+'
+
+
+test_expect_success 'credential_reject calls all helpers' '
+ check reject useless "verbatim one two" <<-\EOF
+ username=foo
+ password=bar
+ --
+ --
+ useless: erase
+ useless: username=foo
+ useless: password=bar
+ verbatim: erase
+ verbatim: username=foo
+ verbatim: password=bar
+ EOF
+'
+
+test_expect_success 'usernames can be preserved' '
+ check fill "verbatim \"\" three" <<-\EOF
+ username=one
+ --
+ username=one
+ password=three
+ --
+ verbatim: get
+ verbatim: username=one
+ EOF
+'
+
+test_expect_success 'usernames can be overridden' '
+ check fill "verbatim two three" <<-\EOF
+ username=one
+ --
+ username=two
+ password=three
+ --
+ verbatim: get
+ verbatim: username=one
+ EOF
+'
+
+test_expect_success 'do not bother completing already-full credential' '
+ check fill "verbatim three four" <<-\EOF
+ username=one
+ password=two
+ --
+ username=one
+ password=two
+ --
+ EOF
+'
+
+# We can't test the basic terminal password prompt here because
+# getpass() tries too hard to find the real terminal. But if our
+# askpass helper is run, we know the internal getpass is working.
+test_expect_success 'empty helper list falls back to internal getpass' '
+ check fill <<-\EOF
+ --
+ username=askpass-username
+ password=askpass-password
+ --
+ askpass: Username:
+ askpass: Password:
+ EOF
+'
+
+test_expect_success 'internal getpass does not ask for known username' '
+ check fill <<-\EOF
+ username=foo
+ --
+ username=foo
+ password=askpass-password
+ --
+ askpass: Password:
+ EOF
+'
+
+test_done
diff --git a/test-credential.c b/test-credential.c
new file mode 100644
index 0000000..dee200e
--- /dev/null
+++ b/test-credential.c
@@ -0,0 +1,38 @@
+#include "cache.h"
+#include "credential.h"
+#include "string-list.h"
+
+static const char usage_msg[] =
+"test-credential <fill|approve|reject> [helper...]";
+
+int main(int argc, const char **argv)
+{
+ const char *op;
+ struct credential c = CREDENTIAL_INIT;
+ int i;
+
+ op = argv[1];
+ if (!op)
+ usage(usage_msg);
+ for (i = 2; i < argc; i++)
+ string_list_append(&c.helpers, argv[i]);
+
+ if (credential_read(&c, stdin) < 0)
+ die("unable to read credential from stdin");
+
+ if (!strcmp(op, "fill")) {
+ credential_fill(&c);
+ if (c.username)
+ printf("username=%s\n", c.username);
+ if (c.password)
+ printf("password=%s\n", c.password);
+ }
+ else if (!strcmp(op, "approve"))
+ credential_approve(&c);
+ else if (!strcmp(op, "reject"))
+ credential_reject(&c);
+ else
+ usage(usage_msg);
+
+ return 0;
+}
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
* [PATCH 04/13] credential: add function for parsing url components
From: Jeff King @ 2011-11-24 11:01 UTC (permalink / raw)
To: git
In-Reply-To: <20111124105801.GA6168@sigill.intra.peff.net>
All of the components of a credential struct can be found in
a URL. For example, the URL:
http://foo:bar@example.com/repo.git
contains:
protocol=http
host=example.com
path=repo.git
username=foo
password=bar
We want to be able to turn URLs into broken-down credential
structs so that we know two things:
1. Which parts of the username/password we still need
2. What the context of the request is (for prompting or
as a key for storing credentials).
This code is based on http_auth_init in http.c, but needed a
few modifications in order to get all of the components that
the credential object is interested in.
Once the http code is switched over to the credential API,
then http_auth_init can just go away.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/technical/api-credentials.txt | 3 ++
credential.c | 52 +++++++++++++++++++++++++++
credential.h | 1 +
3 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/Documentation/technical/api-credentials.txt b/Documentation/technical/api-credentials.txt
index 3061077..0dca0bf 100644
--- a/Documentation/technical/api-credentials.txt
+++ b/Documentation/technical/api-credentials.txt
@@ -62,6 +62,9 @@ Functions
storage helpers of the approval, so that they can store the
result to be used again.
+`credential_from_url`::
+
+ Parse a URL into broken-down credential fields.
Credential Storage Helpers
--------------------------
diff --git a/credential.c b/credential.c
index d50a611..5cc4319 100644
--- a/credential.c
+++ b/credential.c
@@ -2,6 +2,7 @@
#include "credential.h"
#include "string-list.h"
#include "run-command.h"
+#include "url.h"
void credential_init(struct credential *c)
{
@@ -240,3 +241,54 @@ void credential_reject(struct credential *c)
c->password = NULL;
c->approved = 0;
}
+
+void credential_from_url(struct credential *c, const char *url)
+{
+ const char *at, *colon, *cp, *slash, *host, *proto_end;
+
+ credential_clear(c);
+
+ /*
+ * Match one of:
+ * (1) proto://<host>/...
+ * (2) proto://<user>@<host>/...
+ * (3) proto://<user>:<pass>@<host>/...
+ */
+ proto_end = strstr(url, "://");
+ if (!proto_end)
+ return;
+ cp = proto_end + 3;
+ at = strchr(cp, '@');
+ colon = strchr(cp, ':');
+ slash = strchrnul(cp, '/');
+
+ if (!at || slash <= at) {
+ /* Case (1) */
+ host = cp;
+ }
+ else if (!colon || at <= colon) {
+ /* Case (2) */
+ c->username = url_decode_mem(cp, at - cp);
+ host = at + 1;
+ } else {
+ /* Case (3) */
+ c->username = url_decode_mem(cp, colon - cp);
+ c->password = url_decode_mem(colon + 1, at - (colon + 1));
+ host = at + 1;
+ }
+
+ if (proto_end - url > 0)
+ c->protocol = xmemdupz(url, proto_end - url);
+ if (slash - host > 0)
+ c->host = url_decode_mem(host, slash - host);
+ /* Trim leading and trailing slashes from path */
+ while (*slash == '/')
+ slash++;
+ if (*slash) {
+ char *p;
+ c->path = url_decode(slash);
+ p = c->path + strlen(c->path) - 1;
+ while (p > c->path && *p == '/')
+ *p-- = '\0';
+ }
+}
diff --git a/credential.h b/credential.h
index 2ea7d49..8a6d162 100644
--- a/credential.h
+++ b/credential.h
@@ -24,5 +24,6 @@ struct credential {
void credential_reject(struct credential *);
int credential_read(struct credential *, FILE *);
+void credential_from_url(struct credential *, const char *url);
#endif /* CREDENTIAL_H */
--
1.7.7.4.5.gb32a5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox