Git development
 help / color / mirror / Atom feed
* Re: git-clone works with ssh but not with http/https/git
From: Jakub Narebski @ 2008-06-24 12:25 UTC (permalink / raw)
  To: Erez Zilber; +Cc: Johannes Schindelin, Matthias Kestenholz, git
In-Reply-To: <ce513bcc0806240507q58c2a3y5fe8f0e8033353ad@mail.gmail.com>

"Erez Zilber" <erezzi.list@gmail.com> writes:

> I think that I'm using the right repository for git-clone because it's
> the same path that I used for git push:
> 
> [root@kd001 my_test.git]# git-push --all
> ssh://erez.zilber@kites/pub/git/erez.zilber/my_test.git

[...]
> looks like git-update-server-info did some work (added the refs file).
> 
> Back to the client:
> 
> [root@kd001 t]# git-clone http://kites/pub/git/erez.zilber/my_test.git
> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
> Cannot get remote repository information.
> Perhaps git-update-server-info needs to be run there?
> 
> The path is the same path that was used for pushing the repository.
> What did I miss here?

First, obvious question: do you have web server enabled?

Second, less obvious: how this server maps pathname information from
URL into path in the filesystem?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Rogan Dawes @ 2008-06-24 12:19 UTC (permalink / raw)
  To: David Jeske; +Cc: Jakub Narebski, Avery Pennarun, Nicolas Pitre, git
In-Reply-To: <28156.2147582465$1214307807@news.gmane.org>

David Jeske wrote:
> -- Jakub Narebski wrote:
>> If they are using '-f', i.e. force, they should know and be sure what
>> they are doing; it is not much different from 'rm -f *'.
> 
> Sure, no problem. I don't want the ability to "rm -f *". I'm raising my hand
> and saying "I don't want the power to do these things, so just turn off all the
> git commands that could be destructive and give me an alternate way to do the
> workflows I need to do". Just like a normal user on a unix machine doesn't run
> around with the power to rm -f /etc all the time, even though they may be able
> to su to root.
> 
> Let me guess, you're always running euid==0. :)

Do you also ask the gnu coreutils folks to remove the -f option from 
their utilities?

There is a basic assumption that folks that are using tools have at 
least made an attempt to understand what it is that they are doing, 
before e.g. waving a chainsaw around.

One thing that I haven't seen addressed in this thread is the fact that 
if you have a dirty working directory, and you "git reset --hard", 
whatever was dirty (not yet in the index, or committed) will be blown 
away, and no amount of reflog archeology will help you get it back.

Any changes that had been staged in the index WILL exist in the object 
directories as dangling objects, and can be retrieved through judicious 
use of "git fsck" and "git show", but will certainly be a painful 
exercise if there was an extensive set of changes.

Rogan

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Jakub Narebski @ 2008-06-24 12:21 UTC (permalink / raw)
  To: David Jeske; +Cc: Avery Pennarun, Nicolas Pitre, git
In-Reply-To: <willow-jeske-01l5pWdEFEDjCjLX>

David Jeske wrote:
> -- Jakub Narebski wrote:
>>
>> If they are using '-f', i.e. force, they should know and be sure what
>> they are doing; it is not much different from 'rm -f *'.

By the way, reflog (even if expired) would protect you in this
situation; I have checked wrongly that it does not (chronological
vs. reverse chronological order, and not paying attention to
timestamps).

> Sure, no problem. I don't want the ability to "rm -f *". [...]

It is very useful command when deleting larger number of files;
I have "alias rm='rm -i'", and confirming every single file quickly
gets annoying.

> Just like a normal user on a unix machine doesn't run 
> around with the power to rm -f /etc all the time, even though they may be able
> to su to root.

Example was about "rm -f *", i.e. removing contents of current directory;
you should be careful when doing it, for example if you are in currect
repository.
 
Some older versions of UNIX supposedly could hose every hidden file you own
upwards if you did "rm -rf .*", as they matched '..' (parent directory)
against '.*'.

> Let me guess, you're always running euid==0. :)

No.  I almost never login as root, using 'sudo', 'sudo su -', or relying
on applications asking for root credentials if required (for example when
installing new version of git).

Let me guess: no sharp knives in kitchen? ;-P
-- 
Jakub Narebski
Poland

^ permalink raw reply

* git-fetch remote tag to local branch fails
From: Klas Lindberg @ 2008-06-24 12:17 UTC (permalink / raw)
  To: git

Hi

I have a repository "bogustree" that contains tags "test_tag" and
"foo_tag" (both on the same branch, with "test_tag" pointing to an
older commit).
I also have repository "R", which contains "test_tag" from "bogustree"
and a branch "b" created from "test_tag". The active branch in R is
"master".

Now I want to do the following from R:

git-fetch bogustree refs/tags/foo_tag:refs/heads/b

but this fails with the message given below. The interesting part is
that git-fetch tries to write a "non-commit object" and indeed I have
no idea what this thing is (the commit pointed to by "foo_tag" has
commit ID cab0a25388f1884a1ab16bd0d66f877c0b36f1d5). Another
interesting detail is that the command fails and then continues
anyway. That seems wrong in itself.

# remote: Counting objects: 6, done.
# remote: Compressing objects: 100% (2/2), done.
# remote: Total 4 (delta 0), reused 0 (delta 0)
# Unpacking objects: 100% (4/4), done.
# error: Trying to write non-commit object
5f61233bc5dad0f3e25ecea65457f5bb528174d2 to branch refs/heads/B
# From /home/qliklas/bogustree/b/b
#    72f0622..5f61233  foo_tag    -> b
# From /home/qliklas/bogustree/b/b
#  * [new tag]         foo_tag    -> foo_tag
#  * [new tag]         test_tag   -> test_tag

Why can't I fetch like this? The manual page for git-fetch says that

       <refspec>
              The canonical format of a <refspec> parameter is +?<src>:<dst>;
              that is, an optional plus +, followed by the source ref,
              followed by a colon :, followed by the destination ref.

              The remote ref that matches <src> is fetched, and if <dst> is
              not empty string, the local ref that matches it is fast
              forwarded using <src>. Again, if the optional plus + is used,
              the local ref is updated even if it does not result in a fast
              forward update.

so I think it should be OK?

BR / Klas

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Jakub Narebski @ 2008-06-24 12:13 UTC (permalink / raw)
  To: David Jeske; +Cc: Avery Pennarun, Nicolas Pitre, git
In-Reply-To: <200806241322.14224.jnareb@gmail.com>

Jakub Narebski wrote:
> David Jeske wrote:

> > Now, five years down the road, [...] someone does:
> > 
> >  $ git-branch -f customer_A_branch ZZZ
> 
> If they are using '-f', i.e. force, they should know and be sure what
> they are doing; it is not much different from 'rm -f *'.
> 
> If reflog for 'customer_A_branch' expired it would be hard to go back
> to old 'customer_A_branch', and impossible after garbage collector
> pruned history.

Actually it is not true.  In the case of "git branch -f <branch>", which
is the case which wouldn't be covered by protecting reflogs when
deleting branches (saving them to some kind of "attic") git _saves_
old branch pointer to reflog, so "git log -g <branch>" would work
as expected.

The reflog entry looks like the following:

   0c52414d... 80b4c7e5.. A U Thor <author@example.com> 1214306246 +0200 \
	branch: Reset from ZZZ

(where of course there are full SHA-1 of commits, instead of shortened
ones, and everything is in single line, without line continuation.)
 
> What you _should do_, if you want to preserve old 'customer_A_branch'
> pointer is to *tag* it, e.g. something like 'Attic/customer_A_branch';
> if you use annotated tags you can even state why do you want to keep
> old work, and why old work wasn't merged into long-lived branch, and
> why the work was abandoned.

This of course is still valid.

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: git-clone works with ssh but not with http/https/git
From: Erez Zilber @ 2008-06-24 12:07 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Matthias Kestenholz, git
In-Reply-To: <alpine.DEB.1.00.0806241246500.9925@racer>

On Tue, Jun 24, 2008 at 2:48 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Tue, 24 Jun 2008, Erez Zilber wrote:
>
>> On Tue, Jun 24, 2008 at 2:21 PM, Matthias Kestenholz <mk@spinlock.ch> wrote:
>> > On Tue, 2008-06-24 at 14:15 +0300, Erez Zilber wrote:
>> >
>> >> [root@kd001 t]# git-clone http://kites/pub/git/erez.zilber/my_test.git
>> >> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
>> >> Cannot get remote repository information.
>> >> Perhaps git-update-server-info needs to be run there?
>> >>
>> >
>> > Maybe you should run git-update-server-info in the repository on the
>> > server?
>>
>> Forgot to mention - I tried to run git-update-server-info from the
>> repository on the server.
>
> No, you did not.  Otherwise, the message would not appear.  It can be that
> you have _another_ repository on the server, but running
> update-server-info there does not help.

Thanks for the quick answer. I'm sure that something _is_ wrong.
Otherwise, everything should have worked already.

I think that I'm using the right repository for git-clone because it's
the same path that I used for git push:

[root@kd001 my_test.git]# git-push --all
ssh://erez.zilber@kites/pub/git/erez.zilber/my_test.git
erez.zilber@kites's password:
Counting objects: 9, done.
Compressing objects: 100% (5/5), done.
Writing objects: 100% (9/9), 740 bytes, done.
Total 9 (delta 0), reused 0 (delta 0)
To ssh://erez.zilber@kites/pub/git/erez.zilber/my_test.git
 * [new branch]      master -> master

Now, on the server side:

-sh-3.1$ cd my_test.git/
-sh-3.1$ ls info/
exclude
-sh-3.1$ git-update-server-info
-sh-3.1$ ls info/
exclude  refs

looks like git-update-server-info did some work (added the refs file).

Back to the client:

[root@kd001 t]# git-clone http://kites/pub/git/erez.zilber/my_test.git
Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

The path is the same path that was used for pushing the repository.
What did I miss here?

Thanks,
Erez

^ permalink raw reply

* Re: git-clone works with ssh but not with http/https/git
From: Johannes Schindelin @ 2008-06-24 11:48 UTC (permalink / raw)
  To: Erez Zilber; +Cc: Matthias Kestenholz, git
In-Reply-To: <ce513bcc0806240445x6d00323g303f218504d2df53@mail.gmail.com>

Hi,

On Tue, 24 Jun 2008, Erez Zilber wrote:

> On Tue, Jun 24, 2008 at 2:21 PM, Matthias Kestenholz <mk@spinlock.ch> wrote:
> > On Tue, 2008-06-24 at 14:15 +0300, Erez Zilber wrote:
> >
> >> [root@kd001 t]# git-clone http://kites/pub/git/erez.zilber/my_test.git
> >> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
> >> Cannot get remote repository information.
> >> Perhaps git-update-server-info needs to be run there?
> >>
> >
> > Maybe you should run git-update-server-info in the repository on the
> > server?
> 
> Forgot to mention - I tried to run git-update-server-info from the
> repository on the server.

No, you did not.  Otherwise, the message would not appear.  It can be that 
you have _another_ repository on the server, but running 
update-server-info there does not help.

> I got the same results. BTW - if it works OK with ssh, I can only guess 
> that git-update-server-info won't help for http. right?

Wrong again.  ssh transport is special, and does not need the information 
update-server-info provides.

Hth,
Dscho

^ permalink raw reply

* Re: git-clone works with ssh but not with http/https/git
From: Erez Zilber @ 2008-06-24 11:45 UTC (permalink / raw)
  To: Matthias Kestenholz; +Cc: git
In-Reply-To: <1214306517.6441.10.camel@localhost>

On Tue, Jun 24, 2008 at 2:21 PM, Matthias Kestenholz <mk@spinlock.ch> wrote:
> On Tue, 2008-06-24 at 14:15 +0300, Erez Zilber wrote:
>> Hi,
>>
>> I'm able to git-clone a tree using ssh:
>>
>> [root@kd001 t]# git-clone
>> ssh://erez.zilber@kites/pub/git/erez.zilber/my_test.git
>> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
>> erez.zilber@kites's password:
>> remote: Counting objects: 9, done.
>> remote: Compressing objects: 100% (5/5), done.
>> remote: Total 9 (delta 0), reused 0 (delta 0)
>> Receiving objects: 100% (9/9), done.
>>
>> However, it doesn't work with http/https/git:
>>
>> [root@kd001 t]# git-clone http://kites/pub/git/erez.zilber/my_test.git
>> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
>> Cannot get remote repository information.
>> Perhaps git-update-server-info needs to be run there?
>>
>
> Maybe you should run git-update-server-info in the repository on the
> server?

Forgot to mention - I tried to run git-update-server-info from the
repository on the server. I got the same results. BTW - if it works OK
with ssh, I can only guess that git-update-server-info won't help for
http. right?

>
>> [root@kd001 t]# git-clone https://kites/pub/git/erez.zilber/my_test.git
>> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
>> Cannot get remote repository information.
>> Perhaps git-update-server-info needs to be run there?
>
>
> Maybe you should run git-update-server-info in the repository on the
> server?
>
> By the way, try enabling the post-update hook -- it runs
> update-server-info for you automatically after pushing to your server
> (make the script $GIT_DIR/hooks/post-update executable)

Make it executable on the client side before pushing (i.e. in
.git/hooks/post-update) or on the server side after pushing?

>
>> [root@kd001 t]# git-clone git://kites/pub/git/erez.zilber/my_test.git
>> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
>> kites[0: 172.16.1.11]: errno=Connection refused
>> fatal: unable to connect a socket (Connection refused)
>> fetch-pack from 'git://kites/pub/git/erez.zilber/my_test.git' failed.
>>
>
> Is the git daemon running on your server? You need to configure inetd or
> git-daemon yourself, that is not done automatically for you.
>

I ran git-daemon on the server side like this:

sudo git-daemon --base-path=/pub/git/ --export-all --listen=kites

Erez

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 11:29 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Avery Pennarun, Nicolas Pitre, git
In-Reply-To: <200806241322.14224.jnareb@gmail.com>

-- Jakub Narebski wrote:
> If they are using '-f', i.e. force, they should know and be sure what
> they are doing; it is not much different from 'rm -f *'.

Sure, no problem. I don't want the ability to "rm -f *". I'm raising my hand
and saying "I don't want the power to do these things, so just turn off all the
git commands that could be destructive and give me an alternate way to do the
workflows I need to do". Just like a normal user on a unix machine doesn't run
around with the power to rm -f /etc all the time, even though they may be able
to su to root.

Let me guess, you're always running euid==0. :)

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 11:29 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Avery Pennarun, Nicolas Pitre, git
In-Reply-To: <200806241322.14224.jnareb@gmail.com>

-- Jakub Narebski wrote:
> If they are using '-f', i.e. force, they should know and be sure what
> they are doing; it is not much different from 'rm -f *'.

Sure, no problem. I don't want the ability to "rm -f *". I'm raising my hand
and saying "I don't want the power to do these things, so just turn off all the
git commands that could be destructive and give me an alternate way to do the
workflows I need to do". Just like a normal user on a unix machine doesn't run
around with the power to rm -f /etc all the time, even though they may be able
to su to root.

Let me guess, you're always running euid==0. :)

^ permalink raw reply

* Re: git-clone works with ssh but not with http/https/git
From: Matthias Kestenholz @ 2008-06-24 11:21 UTC (permalink / raw)
  To: Erez Zilber; +Cc: git
In-Reply-To: <ce513bcc0806240415h669d1725uf7b6e495995ab459@mail.gmail.com>

On Tue, 2008-06-24 at 14:15 +0300, Erez Zilber wrote:
> Hi,
> 
> I'm able to git-clone a tree using ssh:
> 
> [root@kd001 t]# git-clone
> ssh://erez.zilber@kites/pub/git/erez.zilber/my_test.git
> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
> erez.zilber@kites's password:
> remote: Counting objects: 9, done.
> remote: Compressing objects: 100% (5/5), done.
> remote: Total 9 (delta 0), reused 0 (delta 0)
> Receiving objects: 100% (9/9), done.
> 
> However, it doesn't work with http/https/git:
> 
> [root@kd001 t]# git-clone http://kites/pub/git/erez.zilber/my_test.git
> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
> Cannot get remote repository information.
> Perhaps git-update-server-info needs to be run there?
> 

Maybe you should run git-update-server-info in the repository on the
server?

> [root@kd001 t]# git-clone https://kites/pub/git/erez.zilber/my_test.git
> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
> Cannot get remote repository information.
> Perhaps git-update-server-info needs to be run there?


Maybe you should run git-update-server-info in the repository on the
server?

By the way, try enabling the post-update hook -- it runs
update-server-info for you automatically after pushing to your server
(make the script $GIT_DIR/hooks/post-update executable)

> [root@kd001 t]# git-clone git://kites/pub/git/erez.zilber/my_test.git
> Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
> kites[0: 172.16.1.11]: errno=Connection refused
> fatal: unable to connect a socket (Connection refused)
> fetch-pack from 'git://kites/pub/git/erez.zilber/my_test.git' failed.
> 

Is the git daemon running on your server? You need to configure inetd or
git-daemon yourself, that is not done automatically for you.


-- 
http://spinlock.ch/blog/

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Jakub Narebski @ 2008-06-24 11:22 UTC (permalink / raw)
  To: David Jeske; +Cc: Avery Pennarun, Nicolas Pitre, git
In-Reply-To: <willow-jeske-01l5kwGPFEDjCc7b>

David Jeske wrote:

> To re-ask the same question I asked in my last post, using your ascii
> pictures...
> 
> 
> Let's assume we're here..
> 
> .<---.<---.<---A<---X<---Y    <--- master
>  \
>   \--B<---C                   <--- customer_A_branch <=== HEAD
> 
> 
> And this person and everyone else moves their head pointers back
> to master without merging:

You could simply say: they stop working on 'customer_A_branch' branch
(moving HEAD poter is simply switching to / checking out / working on
different branch).
 
> .<---.<---.<---A<---X<---Y    <--- master              <=== HEAD
>  \
>   \--B<---C                   <--- customer_A_branch
> 
> 
> Now, five years down the road, our tree looks like:
> 
> 
> .<---.<---.<---A<---X<---Y<--.(3 years of changes).--ZZZ  <--- master  <=== HEAD
>  \
>   \--B<---C   <--- customer_A_branch
> 
> And someone does:
> 
> git-branch -f customer_A_branch ZZZ

If they are using '-f', i.e. force, they should know and be sure what
they are doing; it is not much different from 'rm -f *'.

If reflog for 'customer_A_branch' expired it would be hard to go back
to old 'customer_A_branch', and impossible after garbage collector
pruned history.

What you _should do_, if you want to preserve old 'customer_A_branch'
pointer is to *tag* it, e.g. something like 'Attic/customer_A_branch';
if you use annotated tags you can even state why do you want to keep
old work, and why old work wasn't merged into long-lived branch, and
why the work was abandoned.


-- 
Jakub Narebski
Poland

^ permalink raw reply

* git-clone works with ssh but not with http/https/git
From: Erez Zilber @ 2008-06-24 11:15 UTC (permalink / raw)
  To: git

Hi,

I'm able to git-clone a tree using ssh:

[root@kd001 t]# git-clone
ssh://erez.zilber@kites/pub/git/erez.zilber/my_test.git
Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
erez.zilber@kites's password:
remote: Counting objects: 9, done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 9 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (9/9), done.

However, it doesn't work with http/https/git:

[root@kd001 t]# git-clone http://kites/pub/git/erez.zilber/my_test.git
Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

[root@kd001 t]# git-clone https://kites/pub/git/erez.zilber/my_test.git
Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
Cannot get remote repository information.
Perhaps git-update-server-info needs to be run there?

[root@kd001 t]# git-clone git://kites/pub/git/erez.zilber/my_test.git
Initialized empty Git repository in /home/erez.zilber/work/tmp/t/my_test/.git/
kites[0: 172.16.1.11]: errno=Connection refused
fatal: unable to connect a socket (Connection refused)
fetch-pack from 'git://kites/pub/git/erez.zilber/my_test.git' failed.

What's the problem?

Thanks,
Erez

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 10:42 UTC (permalink / raw)
  To: git
In-Reply-To: <willow-jeske-01l5oEsvFEDjCjRW>

As a more practical question, how do I do this workflow illustrated below?

It's sort of similar to the workflow that "git stash" is trying to support,
except that I have a bunch of commits instead of a bunch of
uncommitted-changes.

I pull a repository that looks like this:

.  a<--b<--c  <--master

Then I hack away to this, and then throw my own branch on the end, along with
master:

.  a<--b<--c<--d<--e<--f<--g  <--master (jeske)
.                             <--feature1 (jeske)

While the server looks like this:

.  a<--b<--c<--1<--2<--3  <--master (server)

I want to get my repository to look something like this:

.  a<--b<--c<--1<--2<--3  <--master (jeske)
.           \
.            d<--e<--f<--g   <-- feature1 (jeske)

So I can then do this:

.  a<--b<--c<--1<--2<--3<--zz  <--master (jeske)
.           \
.            d<--e<--f<--g   <-- feature1 (jeske)

..and then push zz onto the server after 3.

..and I want to do it with safe commands that won't leave any dangling
references. (say if I forget to put the feature1 branch on)

How do I do that?

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 10:42 UTC (permalink / raw)
  To: git
In-Reply-To: <willow-jeske-01l5oEsvFEDjCjRW>

As a more practical question, how do I do this workflow illustrated below?

It's sort of similar to the workflow that "git stash" is trying to support,
except that I have a bunch of commits instead of a bunch of
uncommitted-changes.

I pull a repository that looks like this:

.  a<--b<--c  <--master

Then I hack away to this, and then throw my own branch on the end, along with
master:

.  a<--b<--c<--d<--e<--f<--g  <--master (jeske)
.                             <--feature1 (jeske)

While the server looks like this:

.  a<--b<--c<--1<--2<--3  <--master (server)

I want to get my repository to look something like this:

.  a<--b<--c<--1<--2<--3  <--master (jeske)
.           \
.            d<--e<--f<--g   <-- feature1 (jeske)

So I can then do this:

.  a<--b<--c<--1<--2<--3<--zz  <--master (jeske)
.           \
.            d<--e<--f<--g   <-- feature1 (jeske)

..and then push zz onto the server after 3.

..and I want to do it with safe commands that won't leave any dangling
references. (say if I forget to put the feature1 branch on)

How do I do that?

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Fedor Sergeev @ 2008-06-24 10:01 UTC (permalink / raw)
  To: David Jeske; +Cc: git
In-Reply-To: <willow-jeske-01l5lTEoFEDjCVta@brm-avmta-1.central.sun.com>


On Tue, 24 Jun 2008, David Jeske wrote:
> This is mostly moot since I've understood that it's easy to set git to never
> GC. I guess I'm curious about why those GC fields would ever be set to anything
> other than never?

On Tue, 24 Jun 2008, David Jeske wrote:
> My philosophy is simple, I never never
> never want to throw away changes, you shouldn't either. Disks are cheaper than
> programmer hours. I can understand wanting to keep things tidy, so I can
> understand ways to correct the 'easily visible changes', and also avoid pushing
> them to other trees, but I don't understand why git needs to delete things.

It looks like you are severely restricting your own way of thinking about
a source code management as a source code backup system only.

While this might be a valid mindset for a gatekeeper on a public 
repository it way way restrictive for a developer that wants to have a 
system that helps him doing a job.
And, say, for me, for my own job, ability to experiment *safely* and 
effectively, ability to try out different histories is the most valuable
asset that git brings to the world of SCMs.

My collegues that were forced to use Mercurial for their job are really 
unhappy about Mercurial's habbit of not modifying history at all.
After a certain amount of time just looking at the history of an actively 
developed project causes a headache.


When you speak about allowing/disallowing destructive actions you actually
speak about policies.
Different organizations, different repositories have different policies.
And git is very flexible in allowing you to implement all those different
policies as you wish it.

And whether default policy should allow people to experiment freely or not
is a very delicate question, which I would not really have enough courage
to speculate on.

regards,
   Fedor.

P.S. Saying all that, I would really like to have an easy way to tie non-default
policies to repositories so it propagates on clones. It is really helpful
in big organizations. But thats another story.

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: David Jeske @ 2008-06-24 10:24 UTC (permalink / raw)
  To: Fedor Sergeev; +Cc: git
In-Reply-To: <alpine.WNT.1.10.0806241343170.3824@theodor>

On Tue, Jun 24, 2008 at 3:01 AM, Fedor Sergeev <Fedor.Sergeev@sun.com> wrote:
> It looks like you are severely restricting your own way of thinking about
> a source code management as a source code backup system only.
>
> While this might be a valid mindset for a gatekeeper on a public repository
> it way way restrictive for a developer that wants to have a system that
> helps him doing a job.

Odd. I've never been a gatekeeper. I'm just a developer who has burned
himself enough times that I want a tool (i.e. source control) to help
prevent me from ever destroying anything I create. I like that git is
doing nicer things with merge tracking than older systems, and that
it's easier for distributed teams to move changes around in more
interesting ways than "up to the server" and "down from the server".
However, I also want it to provide the guarantee that "if I don't
touch the files in .git, it'll never lose my commits", which sadly
isn't true by default. I'm glad I can easily change the GC policy, but
I question why this isn't the default.

In another discussion about this, one of my coworkers pointed out that
making the GC default "never" would be much safer for new users, and
new users don't really need to worry about collecting things until
their repositories get bigger anyhow.

I also think that it would be simpler to understand for everyone if
every operation which can cause a dangling graph node require the
exact same override method (i.e. -f is fine, the capitalization as in
-d -> -D is fine, some --force or --hard is fine, but currently the
system is using three different methods in three different places)

^ permalink raw reply

* Re: [PATCH 6/7] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.
From: Pierre Habouzit @ 2008-06-24 10:03 UTC (permalink / raw)
  To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin
In-Reply-To: <1214298732-6247-7-git-send-email-madcoder@debian.org>

[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]

On Tue, Jun 24, 2008 at 09:12:11AM +0000, Pierre Habouzit wrote:
> This way, argv[0] isn't clobbered, to the cost of maybe not having a
> resulting NULL terminated argv array.
> 
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>  parse-options.c |    7 ++++---
>  parse-options.h |    2 ++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/parse-options.c b/parse-options.c
> index ee82cf3..a6b5e04 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -246,7 +246,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
>  	memset(ctx, 0, sizeof(*ctx));
>  	ctx->argc = argc - 1;
>  	ctx->argv = argv + 1;
> -	ctx->out  = argv;
> +	ctx->out  = argv + ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
>  	ctx->flags = flags;
>  	strbuf_init(&ctx->buf, 0);
>  }
> @@ -327,10 +327,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
>  
>  int parse_options_end(struct parse_opt_ctx_t *ctx)
>  {
> +	int res = ctx->cpidx + ctx->argc;
>  	memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
> -	ctx->out[ctx->cpidx + ctx->argc] = NULL;
> +	ctx->out[res] = NULL;
>  	strbuf_release(&ctx->buf);
> -	return ctx->cpidx + ctx->argc;
> +	return res + ((ctx->flags & PARSE_OPT_KEEP_ARGV0) != 0);
>  }
>  
>  int parse_options(int argc, const char **argv, const struct option *options,
> diff --git a/parse-options.h b/parse-options.h
> index 14447d5..6745c7d 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -22,6 +22,8 @@ enum parse_opt_type {
>  enum parse_opt_flags {
>  	PARSE_OPT_KEEP_DASHDASH = 1,
>  	PARSE_OPT_STOP_AT_NON_OPTION = 2,
> +	/* using that option, the filtered argv may not be NULL terminated */

  This comment is bogus and shall be stripped, I forgot to…
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: why is git destructive by default? (i suggest it not be!)
From: Jakub Narebski @ 2008-06-24  9:39 UTC (permalink / raw)
  To: David Jeske; +Cc: Jeff King, Avery Pennarun, Nicolas Pitre, git
In-Reply-To: <15381.9593288519$1214297235@news.gmane.org>

"David Jeske" <jeske@google.com> writes:

> This is mostly moot since I've understood that it's easy to set git
> to never GC. I guess I'm curious about why those GC fields would
> ever be set to anything other than never?

Because not everybody has unlimited quota / unlimited disk space?
Besides growing repository, reflogs also grow even if you shitch
between some limited set of commits.

Note however that IIRC reflogs are not enabled by default for bare
repositories, and public repositories should be bare (without working
directory).  But see receive.denyNonFastForwards below.

> -- Jeff King wrote:
> >
> > No. Git keeps the reachable DAG. So if the DAG is part of development
> > that is merged into one of your long running branches, or if you keep
> > around the branch that points to it, it will never go away.
> 
> Right, that's what I thought.
> 
> I'm not primarily concerned with what developers can do to their
> local git repositories. I'm concerned with what the default sync
> operations can let them do to the crown-jewels in the 'central
> organization repositories' which everyone is periodically pushing
> to.
> 
> I like that deleting a branch in your repo does not cause it to be
> deleted in other repos. Presumably in an organization we could
> prevent the central repo from ever accepting branch deletes from
> developers. (without some kind of authorization)
> 
> Does it have the same protection for all operations that can cause
> DAGs to be dangling? For example, if they branch -f" and push the
> branch?

git-config(1)

  receive.denyNonFastForwards::
        If set to true, git-receive-pack will deny a ref update which is
        not a fast forward. Use this to prevent such an update via a push,
        even if that push is forced. This configuration variable is
        set when initializing a shared repository.

That is even more than protection against leaving some commits
dangling.  This makes working on top of published branches safe.

If such all-or-nothing policy is not for you, you can always set-up
hooks, like shown for example in contrib/hooks/update-paranoid

Or you can use different workflow, where maintainer _pulls_ from other
developers or groups of developers, or apply (git-am) patches from
email.  This way if you screw up, it would be your fault for not
having backups ;-)

[...]
> Quite a bit of my fear of losing data came from some issues in the
> git-gui. I'm trying out git on a windows project, and windows-shells
> just don't work right, so I'm using the "Git Gui". It turns out
> right-clicking on a history entry in the gui has no checkout option,

This might be result of the fact that in older versions of git you
could not checkout arbitrary commit.  You now can use so called
"detached HEAD" (when current branch pointer points directly to the
commit, instead of pointing to current branch [name]); note however
that comitting on top of detached HEAD is discouraged.

> and the only option it does have which will let you move the tree to
> that place is "reset --hard".. since this was the easiest thing to
> find in the GUI, I assumed it was the right way to do it, and then
> all my more recent changes disappeared. It doesn't seem to have
> reflog functionality, so I couldn't find any way to get back all my
> changes.

There is always ORIG_HEAD, which predates reflog introduction, and
contains only old "version", as in

  $ git reset --hard ORIG_HEAD


That said, it would be nice if git-gui had some reflog interface.

> [...] The docs clearly explained that it
> will garbage collect dangling refs, and frankly the information
> about how often this happens is buried so deep I had no idea what
> the frequency was.

git-gc(1), section called (suprise, suprise) "Configuration".

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI
From: Jakub Narebski @ 2008-06-24  9:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Lea Wiemann, git
In-Reply-To: <7vtzfjea64.fsf@gitster.siamese.dyndns.org>

On Tue, 24 Jun 2008, Junio C Hamano wrote:

> About the Test::WWW::Mechanize::CGI thing, how widely available is it?
> I do not think it is packaged for Debian nor Ubuntu, for example.

Not very widely; practically you have to install it from CPAN.  
Test::WWW::Mechanize::CGI is at v0.1, WWW::Mechanize::CGI is at v0.3.

But if Test::WWW::Mechanize::CGI is not installed, test would be not 
run.  If you are gitweb developer, then installing locally in $HOME 
from CPAN is I guess viable option; if you are not gitweb developer, we 
still have t/t9500-gitweb-standalone-no-errors.sh


The whole point of using those packages was that it makes it _easy_
to write those tests.  This consist of two parts: 

1). running gitweb as if it was CGI application, which otherwise would 
either require deep knowledge of how CGI application is invoked (what 
it is in WWW::Mechanize::CGI and required dependence 
HTTP::Request::AsCGI which I think does all the work) or working
web server, done a la "git instaweb",

2.) accessing and testing gitweb output (what Test::WWW::Mechanize,
WWW::Mechanize does; we could do the same with LWP* modules from 
libwww-perl / perl-libwww-perl package and Test::More from Perl, but
it would be repeating Test::WWW::Mechanize work).
-- 
Jakub Narebski
Poland

^ permalink raw reply

* [PATCH 5/7] parse-opt: fake short strings for callers to believe in.
From: Pierre Habouzit @ 2008-06-24  9:12 UTC (permalink / raw)
  To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1214298732-6247-5-git-send-email-madcoder@debian.org>

If we begin to parse -abc and that the parser knew about -a and -b, it
will fake a -c switch for the caller to deal with.

Of course in the case of -acb (supposing -c is not taking an argument) the
caller will have to be especially clever to do the same thing. We could
think about exposing an API to do so if it's really needed, but oh well...

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   12 ++++++++++++
 parse-options.h |   12 ++++++++++++
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 90935f3..ee82cf3 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -248,6 +248,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	ctx->argv = argv + 1;
 	ctx->out  = argv;
 	ctx->flags = flags;
+	strbuf_init(&ctx->buf, 0);
 }
 
 static int usage_with_options_internal(const char * const *,
@@ -257,6 +258,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
                        const struct option *options,
                        const char * const usagestr[])
 {
+	/* we must reset ->opt, unknown short option leave it dangling */
+	ctx->opt = NULL;
+
 	for (; ctx->argc; ctx->argc--, ctx->argv++) {
 		const char *arg = ctx->argv[0];
 
@@ -286,6 +290,13 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 				case -1:
 					return parse_options_usage(usagestr, options);
 				case -2:
+					/* fake a short option thing to hide the fact that we may have
+					 * started to parse aggregated stuff
+					 */
+					strbuf_reset(&ctx->buf);
+					strbuf_addch(&ctx->buf, '-');
+					strbuf_addstr(&ctx->buf, ctx->opt);
+					*ctx->argv = ctx->buf.buf;
 					return PARSE_OPT_UNKNOWN;
 				}
 			}
@@ -318,6 +329,7 @@ int parse_options_end(struct parse_opt_ctx_t *ctx)
 {
 	memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
 	ctx->out[ctx->cpidx + ctx->argc] = NULL;
+	strbuf_release(&ctx->buf);
 	return ctx->cpidx + ctx->argc;
 }
 
diff --git a/parse-options.h b/parse-options.h
index 9da5e8c..14447d5 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -1,6 +1,8 @@
 #ifndef PARSE_OPTIONS_H
 #define PARSE_OPTIONS_H
 
+#include "strbuf.h"
+
 enum parse_opt_type {
 	/* special types */
 	OPTION_END,
@@ -119,12 +121,18 @@ enum {
 	PARSE_OPT_UNKNOWN,
 };
 
+/*
+ * It's okay for the caller to consume argv/argc in the usual way.
+ * Other fields of that structure are private to parse-options and should not
+ * be modified in any way.
+ */
 struct parse_opt_ctx_t {
 	const char **argv;
 	const char **out;
 	int argc, cpidx;
 	const char *opt;
 	int flags;
+	struct strbuf buf;
 };
 
 extern int parse_options_usage(const char * const *usagestr,
@@ -133,6 +141,10 @@ extern int parse_options_usage(const char * const *usagestr,
 extern void parse_options_start(struct parse_opt_ctx_t *ctx,
                                 int argc, const char **argv, int flags);
 
+/* Warning: you cannot keep pointers to ctx->argv during the parse
+ *          because some "option strings" are faked. It's okay to use
+ *          ctx->argv after a parse_options_end obviously
+ */
 extern int parse_options_step(struct parse_opt_ctx_t *ctx,
                               const struct option *options,
                               const char * const usagestr[]);
-- 
1.5.6.110.g736c7.dirty

^ permalink raw reply related

* [PATCH 7/7] Migrate git-blame to parse-option partially.
From: Pierre Habouzit @ 2008-06-24  9:12 UTC (permalink / raw)
  To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1214298732-6247-7-git-send-email-madcoder@debian.org>

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

  This patch is not really nice yet, but shows what we can do on top of the
  rest of the series. I shamelessly "stole" most of the code from Linus'
  patch.

  The nasty strdup can be removed safely using my setup_revisions split into
  setup_revisions and parse_revisions, by initializing revisions _before_
  actualy option parsing, making parse_revisions incremental (IOW eating
  options one at a time only) and do the whole thing in one single pass.

  IOW, one would see a inner loop like:

+   for (;;) {
+       int res;
+
+       switch (parse_options_step(&ctx, options, blame_opt_usage)) {
+       case PARSE_OPT_HELP:
+           exit(129);
+       case PARSE_OPT_DONE:
+           goto parse_done;
+       }
+
+       if (!strcmp(ctx.argv[0], "--reverse")) {
+           reverse = 1;
+           ctx.argv[0] = "--children";
+       }
+       res = parse_revisions(&revs, ctx.argc, ctx.argv);
+       if (res <= 0) {
+           if (res == 0)
+               error("unknown option %s", ctx.argv[0]);
+           usage_with_options(options, blame_opt_usage);
+       }
+       ctx.argv += j;
+       ctx.argc -= j;
+   }

  But despite being quite ugly (because of the cpidx fiddling and the strdup),
  this patch works as intended.

 builtin-blame.c |  195 ++++++++++++++++++++++++++++---------------------------
 1 files changed, 100 insertions(+), 95 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index cf41511..632a28d 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -18,24 +18,11 @@
 #include "cache-tree.h"
 #include "path-list.h"
 #include "mailmap.h"
+#include "parse-options.h"
 
-static char blame_usage[] =
-"git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
-"  -c                  Use the same output mode as git-annotate (Default: off)\n"
-"  -b                  Show blank SHA-1 for boundary commits (Default: off)\n"
-"  -l                  Show long commit SHA1 (Default: off)\n"
-"  --root              Do not treat root commits as boundaries (Default: off)\n"
-"  -t                  Show raw timestamp (Default: off)\n"
-"  -f, --show-name     Show original filename (Default: auto)\n"
-"  -n, --show-number   Show original linenumber (Default: off)\n"
-"  -s                  Suppress author name and timestamp (Default: off)\n"
-"  -p, --porcelain     Show in a format designed for machine consumption\n"
-"  -w                  Ignore whitespace differences\n"
-"  -L n,m              Process only line range n,m, counting from 1\n"
-"  -M, -C              Find line movements within and across files\n"
-"  --incremental       Show blame entries as we find them, incrementally\n"
-"  --contents file     Use <file>'s contents as the final image\n"
-"  -S revs-file        Use revisions from revs-file instead of calling git-rev-list\n";
+static char blame_usage[] = "git-blame [options] [--] file";
+
+static const char *blame_opt_usage[] = { blame_usage, NULL };
 
 static int longest_file;
 static int longest_author;
@@ -2219,6 +2206,50 @@ static const char *prepare_initial(struct scoreboard *sb)
    return final_commit_name;
 }
 
+static int blame_copy_callback(const struct option *option, const char *arg, int unset)
+{
+	int *opt = option->value;
+
+	/*
+	 * -C enables copy from removed files;
+	 * -C -C enables copy from existing files, but only
+	 *       when blaming a new file;
+	 * -C -C -C enables copy from existing files for
+	 *          everybody
+	 */
+	if (*opt & PICKAXE_BLAME_COPY_HARDER)
+		*opt |= PICKAXE_BLAME_COPY_HARDEST;
+	if (*opt & PICKAXE_BLAME_COPY)
+		*opt |= PICKAXE_BLAME_COPY_HARDER;
+	*opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
+
+	if (arg)
+		blame_copy_score = parse_score(arg);
+	return 0;
+}
+
+static int blame_move_callback(const struct option *option, const char *arg, int unset)
+{
+	int *opt = option->value;
+
+	*opt |= PICKAXE_BLAME_MOVE;
+
+	if (arg)
+		blame_move_score = parse_score(arg);
+	return 0;
+}
+
+static int blame_bottomtop_callback(const struct option *option, const char *arg, int unset)
+{
+	const char **bottomtop = option->value;
+	if (!arg)
+		return -1;
+	if (*bottomtop)
+		die("More than one '-L n,m' option given");
+	*bottomtop = arg;
+	return 0;
+}
+
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -2226,98 +2257,72 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	struct scoreboard sb;
 	struct origin *o;
 	struct blame_entry *ent;
-	int i, seen_dashdash, unk, opt;
+	int i, seen_dashdash, unk;
 	long bottom, top, lno;
-	int output_option = 0;
-	int show_stats = 0;
-	const char *revs_file = NULL;
 	const char *final_commit_name = NULL;
 	enum object_type type;
-	const char *bottomtop = NULL;
-	const char *contents_from = NULL;
+
+	static const char *bottomtop = NULL;
+	static int output_option = 0, opt = 0;
+	static int show_stats = 0;
+	static const char *revs_file = NULL;
+	static const char *contents_from = NULL;
+	static const struct option options[] = {
+		OPT_BOOLEAN(0, "incremental", &incremental, "Show blame entries as we find them, incrementally"),
+		OPT_BOOLEAN('b', NULL, &blank_boundary, "Show blank SHA-1 for boundary commits (Default: off)"),
+		OPT_BOOLEAN(0, "root", &show_root, "Do not treat root commits as boundaries (Default: off)"),
+		OPT_BOOLEAN(0, "show-stats", &show_stats, "Show work cost statistics"),
+		OPT_BIT(0, "score-debug", &output_option, "Show output score for blame entries", OUTPUT_SHOW_SCORE),
+		OPT_BIT('f', "show-name", &output_option, "Show original filename (Default: auto)", OUTPUT_SHOW_NAME),
+		OPT_BIT('n', "show-number", &output_option, "Show original linenumber (Default: off)", OUTPUT_SHOW_NUMBER),
+		OPT_BIT('p', "porcelain", &output_option, "Show in a format designed for machine consumption", OUTPUT_PORCELAIN),
+		OPT_BIT('c', NULL, &output_option, "Use the same output mode as git-annotate (Default: off)", OUTPUT_ANNOTATE_COMPAT),
+		OPT_BIT('t', NULL, &output_option, "Show raw timestamp (Default: off)", OUTPUT_RAW_TIMESTAMP),
+		OPT_BIT('l', NULL, &output_option, "Show long commit SHA1 (Default: off)", OUTPUT_LONG_OBJECT_NAME),
+		OPT_BIT('s', NULL, &output_option, "Suppress author name and timestamp (Default: off)", OUTPUT_NO_AUTHOR),
+		OPT_BIT('w', NULL, &xdl_opts, "Ignore whitespace differences", XDF_IGNORE_WHITESPACE),
+		OPT_STRING('S', NULL, &revs_file, "file", "Use revisions from <file> instead of calling git-rev-list"),
+		OPT_STRING(0, "contents", &contents_from, "file", "Use <file>'s contents as the final image"),
+		{ OPTION_CALLBACK, 'C', NULL, &opt, "score", "Find line copies within and across files", PARSE_OPT_OPTARG, blame_copy_callback },
+		{ OPTION_CALLBACK, 'M', NULL, &opt, "score", "Find line movements within and across files", PARSE_OPT_OPTARG, blame_move_callback },
+		OPT_CALLBACK('L', NULL, &bottomtop, "n,m", "Process only line range n,m, counting from 1", blame_bottomtop_callback),
+		OPT_END()
+	};
+
+	struct parse_opt_ctx_t ctx;
 
 	cmd_is_annotate = !strcmp(argv[0], "annotate");
 
 	git_config(git_blame_config, NULL);
 	save_commit_buffer = 0;
 
-	opt = 0;
+	parse_options_start(&ctx, argc, argv, PARSE_OPT_KEEP_DASHDASH |
+	                    PARSE_OPT_KEEP_ARGV0);
+	for (;;) {
+		switch (parse_options_step(&ctx, options, blame_opt_usage)) {
+		case PARSE_OPT_HELP:
+			exit(129);
+		case PARSE_OPT_DONE:
+			goto parse_done;
+		}
+
+		if (!strcmp(ctx.argv[0], "--reverse")) {
+			ctx.out[ctx.cpidx++] = "--children";
+			reverse = 1;
+		} else {
+			ctx.out[ctx.cpidx++] = strdup(ctx.argv[0]);
+		}
+		ctx.argv++;
+		ctx.argc--;
+	}
+parse_done:
+	argc = parse_options_end(&ctx);
+
 	seen_dashdash = 0;
 	for (unk = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (*arg != '-')
 			break;
-		else if (!strcmp("-b", arg))
-			blank_boundary = 1;
-		else if (!strcmp("--root", arg))
-			show_root = 1;
-		else if (!strcmp("--reverse", arg)) {
-			argv[unk++] = "--children";
-			reverse = 1;
-		}
-		else if (!strcmp(arg, "--show-stats"))
-			show_stats = 1;
-		else if (!strcmp("-c", arg))
-			output_option |= OUTPUT_ANNOTATE_COMPAT;
-		else if (!strcmp("-t", arg))
-			output_option |= OUTPUT_RAW_TIMESTAMP;
-		else if (!strcmp("-l", arg))
-			output_option |= OUTPUT_LONG_OBJECT_NAME;
-		else if (!strcmp("-s", arg))
-			output_option |= OUTPUT_NO_AUTHOR;
-		else if (!strcmp("-w", arg))
-			xdl_opts |= XDF_IGNORE_WHITESPACE;
-		else if (!strcmp("-S", arg) && ++i < argc)
-			revs_file = argv[i];
-		else if (!prefixcmp(arg, "-M")) {
-			opt |= PICKAXE_BLAME_MOVE;
-			blame_move_score = parse_score(arg+2);
-		}
-		else if (!prefixcmp(arg, "-C")) {
-			/*
-			 * -C enables copy from removed files;
-			 * -C -C enables copy from existing files, but only
-			 *       when blaming a new file;
-			 * -C -C -C enables copy from existing files for
-			 *          everybody
-			 */
-			if (opt & PICKAXE_BLAME_COPY_HARDER)
-				opt |= PICKAXE_BLAME_COPY_HARDEST;
-			if (opt & PICKAXE_BLAME_COPY)
-				opt |= PICKAXE_BLAME_COPY_HARDER;
-			opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
-			blame_copy_score = parse_score(arg+2);
-		}
-		else if (!prefixcmp(arg, "-L")) {
-			if (!arg[2]) {
-				if (++i >= argc)
-					usage(blame_usage);
-				arg = argv[i];
-			}
-			else
-				arg += 2;
-			if (bottomtop)
-				die("More than one '-L n,m' option given");
-			bottomtop = arg;
-		}
-		else if (!strcmp("--contents", arg)) {
-			if (++i >= argc)
-				usage(blame_usage);
-			contents_from = argv[i];
-		}
-		else if (!strcmp("--incremental", arg))
-			incremental = 1;
-		else if (!strcmp("--score-debug", arg))
-			output_option |= OUTPUT_SHOW_SCORE;
-		else if (!strcmp("-f", arg) ||
-			 !strcmp("--show-name", arg))
-			output_option |= OUTPUT_SHOW_NAME;
-		else if (!strcmp("-n", arg) ||
-			 !strcmp("--show-number", arg))
-			output_option |= OUTPUT_SHOW_NUMBER;
-		else if (!strcmp("-p", arg) ||
-			 !strcmp("--porcelain", arg))
-			output_option |= OUTPUT_PORCELAIN;
 		else if (!strcmp("--", arg)) {
 			seen_dashdash = 1;
 			i++;
-- 
1.5.6.110.g736c7.dirty

^ permalink raw reply related

* [PATCH 6/7] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.
From: Pierre Habouzit @ 2008-06-24  9:12 UTC (permalink / raw)
  To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1214298732-6247-6-git-send-email-madcoder@debian.org>

This way, argv[0] isn't clobbered, to the cost of maybe not having a
resulting NULL terminated argv array.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |    7 ++++---
 parse-options.h |    2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index ee82cf3..a6b5e04 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -246,7 +246,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	memset(ctx, 0, sizeof(*ctx));
 	ctx->argc = argc - 1;
 	ctx->argv = argv + 1;
-	ctx->out  = argv;
+	ctx->out  = argv + ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
 	ctx->flags = flags;
 	strbuf_init(&ctx->buf, 0);
 }
@@ -327,10 +327,11 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 
 int parse_options_end(struct parse_opt_ctx_t *ctx)
 {
+	int res = ctx->cpidx + ctx->argc;
 	memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
-	ctx->out[ctx->cpidx + ctx->argc] = NULL;
+	ctx->out[res] = NULL;
 	strbuf_release(&ctx->buf);
-	return ctx->cpidx + ctx->argc;
+	return res + ((ctx->flags & PARSE_OPT_KEEP_ARGV0) != 0);
 }
 
 int parse_options(int argc, const char **argv, const struct option *options,
diff --git a/parse-options.h b/parse-options.h
index 14447d5..6745c7d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -22,6 +22,8 @@ enum parse_opt_type {
 enum parse_opt_flags {
 	PARSE_OPT_KEEP_DASHDASH = 1,
 	PARSE_OPT_STOP_AT_NON_OPTION = 2,
+	/* using that option, the filtered argv may not be NULL terminated */
+	PARSE_OPT_KEEP_ARGV0 = 4,
 };
 
 enum parse_opt_option_flags {
-- 
1.5.6.110.g736c7.dirty

^ permalink raw reply related

* [PATCH 1/7] parse-opt: have parse_options_{start,end}.
From: Pierre Habouzit @ 2008-06-24  9:12 UTC (permalink / raw)
  To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1214298732-6247-1-git-send-email-madcoder@debian.org>

Make the struct optparse_t public under the better name parse_opt_ctx_t.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   69 +++++++++++++++++++++++++++++++------------------------
 parse-options.h |   16 ++++++++++++
 2 files changed, 55 insertions(+), 30 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index b8bde2b..774e647 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -4,14 +4,7 @@
 #define OPT_SHORT 1
 #define OPT_UNSET 2
 
-struct optparse_t {
-	const char **argv;
-	const char **out;
-	int argc, cpidx;
-	const char *opt;
-};
-
-static inline const char *get_arg(struct optparse_t *p)
+static inline const char *get_arg(struct parse_opt_ctx_t *p)
 {
 	if (p->opt) {
 		const char *res = p->opt;
@@ -37,7 +30,7 @@ static int opterror(const struct option *opt, const char *reason, int flags)
 	return error("option `%s' %s", opt->long_name, reason);
 }
 
-static int get_value(struct optparse_t *p,
+static int get_value(struct parse_opt_ctx_t *p,
                      const struct option *opt, int flags)
 {
 	const char *s, *arg;
@@ -131,7 +124,7 @@ static int get_value(struct optparse_t *p,
 	}
 }
 
-static int parse_short_opt(struct optparse_t *p, const struct option *options)
+static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
 {
 	for (; options->type != OPTION_END; options++) {
 		if (options->short_name == *p->opt) {
@@ -142,7 +135,7 @@ static int parse_short_opt(struct optparse_t *p, const struct option *options)
 	return error("unknown switch `%c'", *p->opt);
 }
 
-static int parse_long_opt(struct optparse_t *p, const char *arg,
+static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
                           const struct option *options)
 {
 	const char *arg_end = strchr(arg, '=');
@@ -247,45 +240,63 @@ void check_typos(const char *arg, const struct option *options)
 	}
 }
 
+void parse_options_start(struct parse_opt_ctx_t *ctx,
+                         int argc, const char **argv, int flags)
+{
+	memset(ctx, 0, sizeof(*ctx));
+	ctx->argc = argc - 1;
+	ctx->argv = argv + 1;
+	ctx->out  = argv;
+	ctx->flags = flags;
+}
+
+int parse_options_end(struct parse_opt_ctx_t *ctx)
+{
+	memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
+	ctx->out[ctx->cpidx + ctx->argc] = NULL;
+	return ctx->cpidx + ctx->argc;
+}
+
 static NORETURN void usage_with_options_internal(const char * const *,
                                                  const struct option *, int);
 
 int parse_options(int argc, const char **argv, const struct option *options,
                   const char * const usagestr[], int flags)
 {
-	struct optparse_t args = { argv + 1, argv, argc - 1, 0, NULL };
+	struct parse_opt_ctx_t ctx;
 
-	for (; args.argc; args.argc--, args.argv++) {
-		const char *arg = args.argv[0];
+	parse_options_start(&ctx, argc, argv, flags);
+	for (; ctx.argc; ctx.argc--, ctx.argv++) {
+		const char *arg = ctx.argv[0];
 
 		if (*arg != '-' || !arg[1]) {
-			if (flags & PARSE_OPT_STOP_AT_NON_OPTION)
+			if (ctx.flags & PARSE_OPT_STOP_AT_NON_OPTION)
 				break;
-			args.out[args.cpidx++] = args.argv[0];
+			ctx.out[ctx.cpidx++] = ctx.argv[0];
 			continue;
 		}
 
 		if (arg[1] != '-') {
-			args.opt = arg + 1;
-			if (*args.opt == 'h')
+			ctx.opt = arg + 1;
+			if (*ctx.opt == 'h')
 				usage_with_options(usagestr, options);
-			if (parse_short_opt(&args, options) < 0)
+			if (parse_short_opt(&ctx, options) < 0)
 				usage_with_options(usagestr, options);
-			if (args.opt)
+			if (ctx.opt)
 				check_typos(arg + 1, options);
-			while (args.opt) {
-				if (*args.opt == 'h')
+			while (ctx.opt) {
+				if (*ctx.opt == 'h')
 					usage_with_options(usagestr, options);
-				if (parse_short_opt(&args, options) < 0)
+				if (parse_short_opt(&ctx, options) < 0)
 					usage_with_options(usagestr, options);
 			}
 			continue;
 		}
 
 		if (!arg[2]) { /* "--" */
-			if (!(flags & PARSE_OPT_KEEP_DASHDASH)) {
-				args.argc--;
-				args.argv++;
+			if (!(ctx.flags & PARSE_OPT_KEEP_DASHDASH)) {
+				ctx.argc--;
+				ctx.argv++;
 			}
 			break;
 		}
@@ -294,13 +305,11 @@ int parse_options(int argc, const char **argv, const struct option *options,
 			usage_with_options_internal(usagestr, options, 1);
 		if (!strcmp(arg + 2, "help"))
 			usage_with_options(usagestr, options);
-		if (parse_long_opt(&args, arg + 2, options))
+		if (parse_long_opt(&ctx, arg + 2, options))
 			usage_with_options(usagestr, options);
 	}
 
-	memmove(args.out + args.cpidx, args.argv, args.argc * sizeof(*args.out));
-	args.out[args.cpidx + args.argc] = NULL;
-	return args.cpidx + args.argc;
+	return parse_options_end(&ctx);
 }
 
 #define USAGE_OPTS_WIDTH 24
diff --git a/parse-options.h b/parse-options.h
index 4ee443d..db6c986 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -111,6 +111,22 @@ extern int parse_options(int argc, const char **argv,
 extern NORETURN void usage_with_options(const char * const *usagestr,
                                         const struct option *options);
 
+/*----- incremantal advanced APIs -----*/
+
+struct parse_opt_ctx_t {
+	const char **argv;
+	const char **out;
+	int argc, cpidx;
+	const char *opt;
+	int flags;
+};
+
+extern void parse_options_start(struct parse_opt_ctx_t *ctx,
+                                int argc, const char **argv, int flags);
+
+extern int parse_options_end(struct parse_opt_ctx_t *ctx);
+
+
 /*----- some often used options -----*/
 extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
 extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
-- 
1.5.6.110.g736c7.dirty

^ permalink raw reply related

* [PATCH 4/7] parse-opt: do not pring errors on unknown options, return -2 intead.
From: Pierre Habouzit @ 2008-06-24  9:12 UTC (permalink / raw)
  To: git; +Cc: torvalds, gitster, peff, Johannes.Schindelin, Pierre Habouzit
In-Reply-To: <1214298732-6247-4-git-send-email-madcoder@debian.org>

This way we can catch "unknown" options more easily.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |   43 ++++++++++++++++++++++++++++++-------------
 1 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 71b3476..90935f3 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -94,14 +94,14 @@ static int get_value(struct parse_opt_ctx_t *p,
 
 	case OPTION_CALLBACK:
 		if (unset)
-			return (*opt->callback)(opt, NULL, 1);
+			return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
 		if (opt->flags & PARSE_OPT_NOARG)
-			return (*opt->callback)(opt, NULL, 0);
+			return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
 		if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
-			return (*opt->callback)(opt, NULL, 0);
+			return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
 		if (!arg)
 			return opterror(opt, "requires a value", flags);
-		return (*opt->callback)(opt, get_arg(p), 0);
+		return (*opt->callback)(opt, get_arg(p), 0) ? (-1) : 0;
 
 	case OPTION_INTEGER:
 		if (unset) {
@@ -132,7 +132,7 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
 			return get_value(p, options, OPT_SHORT);
 		}
 	}
-	return error("unknown switch `%c'", *p->opt);
+	return -2;
 }
 
 static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
@@ -217,7 +217,7 @@ is_abbreviated:
 			abbrev_option->long_name);
 	if (abbrev_option)
 		return get_value(p, abbrev_option, abbrev_flags);
-	return error("unknown option `%s'", arg);
+	return -2;
 }
 
 void check_typos(const char *arg, const struct option *options)
@@ -271,15 +271,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			ctx->opt = arg + 1;
 			if (*ctx->opt == 'h')
 				return parse_options_usage(usagestr, options);
-			if (parse_short_opt(ctx, options) < 0)
-				usage_with_options(usagestr, options);
+			switch (parse_short_opt(ctx, options)) {
+			case -1:
+				return parse_options_usage(usagestr, options);
+			case -2:
+				return PARSE_OPT_UNKNOWN;
+			}
 			if (ctx->opt)
 				check_typos(arg + 1, options);
 			while (ctx->opt) {
 				if (*ctx->opt == 'h')
 					return parse_options_usage(usagestr, options);
-				if (parse_short_opt(ctx, options) < 0)
-					usage_with_options(usagestr, options);
+				switch (parse_short_opt(ctx, options)) {
+				case -1:
+					return parse_options_usage(usagestr, options);
+				case -2:
+					return PARSE_OPT_UNKNOWN;
+				}
 			}
 			continue;
 		}
@@ -296,8 +304,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 			return usage_with_options_internal(usagestr, options, 1);
 		if (!strcmp(arg + 2, "help"))
 			return parse_options_usage(usagestr, options);
-		if (parse_long_opt(ctx, arg + 2, options))
-			usage_with_options(usagestr, options);
+		switch (parse_long_opt(ctx, arg + 2, options)) {
+		case -1:
+			return parse_options_usage(usagestr, options);
+		case -2:
+			return PARSE_OPT_UNKNOWN;
+		}
 	}
 	return PARSE_OPT_DONE;
 }
@@ -321,7 +333,12 @@ int parse_options(int argc, const char **argv, const struct option *options,
 	case PARSE_OPT_DONE:
 		break;
 	default: /* PARSE_OPT_UNKNOWN */
-		abort(); /* unreached yet */
+		if (ctx.argv[0][1] == '-') {
+			error("unknown option `%s'", ctx.argv[0] + 2);
+		} else {
+			error("unknown switch `%c'", *ctx.opt);
+		}
+		usage_with_options(usagestr, options);
 	}
 
 	return parse_options_end(&ctx);
-- 
1.5.6.110.g736c7.dirty

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox