* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Ronan Keryell @ 2012-02-07 17:27 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <201202071531.08385.jnareb@gmail.com>
>>>>> On Tue, 7 Feb 2012 15:31:07 +0100, Jakub Narebski <jnareb@gmail.com> said:
>> Agreed, but AFAICS (and modulo the addition of pre-rewrite and
>> pre/post-push hooks mentioned earlier) all of the things
>> discussed so far in this thread can be implemented as hooks.
Jakub> That would be nice.
Jakub> And the new hooks (pre-rewrite, pre/post-push) would be
Jakub> useful anyway, I think.
Yes, to deal with file metadata in git for example. :-)
--
Ronan KERYELL |\/ Phone: +1 408 844 4720
Wild Systems / HPC Project, Inc. |/) Cell: +33 613 143 766
5201 Great America Parkway #3241 K Ronan.Keryell@hpc-project.com
Santa Clara, CA 95054 |\ skype:keryell
USA | \ http://hpc-project.com
^ permalink raw reply
* Re: [PATCH 0/2] config includes, take 2
From: Jeff King @ 2012-02-07 17:30 UTC (permalink / raw)
To: David Aguilar; +Cc: git
In-Reply-To: <CAJDDKr5yiKvNnpVV29jFK1Z1yuUnA-=dn0yMB8iW9y53vRGDHQ@mail.gmail.com>
On Tue, Feb 07, 2012 at 02:05:34AM -0800, David Aguilar wrote:
> I won't claim it's the best solution, but this has worked extremely
> well in practice:
>
> https://github.com/git-cola/git-cola/blob/master/cola/gitcfg.py
>
> Here's the problem. I need to know which config items are "user"
> config (~/.gitconfig), which are "repo" config (.git/config), and
> which are "system" config (/etc/gitconfig).
I'm not a git cola user. But from peeking at that code and a brief
"apt-get install git-cola && git cola", it looks like the point is to
show a dialog with per-repo and per-user settings, let the user tweak
them, and save them back to the appropriate spot.
But how does that interact with includes? Let's imagine I do this:
git config --global include.path .gitidentity
git config -f ~/.gitidentity user.name "Jeff King"
Without having git-cola expand includes, your call to "git config
--global --list" will not show the user.name variable, and it will
appear blank. Which is not great.
If git-cola does expand includes, then the name will appear. But what
will happen when I modify that field and try to save it? It will save it
using "git config --global", putting it into ~/.gitconfig. So now you'll
have two names in your config, one in .gitconfig and one in
.gitidentity.
And which one will take precedence? That depends on what was in your
config file before. If there was a [user] section before the include
already in your .gitconfig, then the newly written value will go in that
section _before_ the include, and the include will still take
precedence. Otherwise, a new section will be written _after_ the
include, and that will take precedence.
So...yuck. It's kind of a complex and messy situation. But the important
thing is that when you are looking at a bidirectional editing situation
like this, blindly expanding includes is not going to solve the problem.
It's just going to paper over the complexity. In the long term,
something like git-cola that is trying to present and edit config to the
user would need to learn about includes, track the sources of each
variable, and then edit appropriately.
You can do that with my current patch by manually following include.path
directives. However, that's a pain. Git-config could potentially help
with that (and even simplify the current code) by allowing something
like:
$ git config --list-with-sources
/home/peff/.gitconfig user.name=Jeff King
/home/peff/.gitconfig user.email=peff@peff.net
.git/config core.repositoryformatversion=0
.git/config core.bare=false
[etc]
(you would use the "-z" form, of course, and the filenames would be
NUL-separated, but I made up a human-readable output format above for
illustration purposes).
And then it would be natural to mention included files there, and you
could actually modify the file that contains the value in question. Of
course, you might not have _access_ to the file in question. You could
be pulling in options from a shared config file, and the right thing to
do is not to edit the shared file, but to override it. And that raises
the question of where, and how to tell git-config to make sure that the
option goes _after_ the include.
So fundamentally, includes make the idea of "overwrite this value" much
more complex. It's possible that git-config should learn all of this
complexity, and that git cola could rely on git-config being smart. But
I was rather hoping that most of the automated tools could remain simple
and stupid, and that people who wanted to do complex things with
includes would be left to their own devices (i.e., using $EDITOR, or
smartly using "git config -f" on their individual files).
> Figuring out the repo config is tricky. You can't just say "git
> config --list" because that includes the user config stuff in
> ~/.gitconfig.
>
> Figuring out the user config is easy because you can say "git config
> --global --list". This is inconsistent with the behavior for "git
> config --list" because it does not include the --system config, which
> one would expect given the overlay semantics.
I think you are mistaking a lack of source options for "look at the repo
config". But without providing a source, the request is "look everywhere
that git knows about". With "--global", it's about "look at this one
file" (just as "--system" is about "look at this other file"). So I
think what you are really lamenting is the lack of "git config --repo",
which would basically be:
git config --list --file="$(git rev-parse --git-dir)/config"
That might be something we could improve in the documentation.
> The generic interface for getting a concise listing of values from
> these sources is to use "git config -f" on ~/.gitconfig, .git/config,
> and $(prefix)/etc/gitconfig.
Right. And --global and --system are basically just synonyms for the
prior two.
> git config --global and git config --system are both consistent in
> that they return just the information relevant to them. Is --global
> just a shortcut for "-f ~/.gitconfig"? If yes, then does that mean
> "git config --global" will not honor includes? If it is not a
> shortcut, does that mean that "git config --global" and "git config -f
> ~/.gitconfig" are not really the same thing? The documentation does
> lead one to believe that they should be equivalent...
Yes, it is a shortcut, and it follows the same code path (i.e., we
expand "--global" into "-f ~/.gitconfig" early on).
> > Yes, that's a general problem of adding new command-line options to turn
> > features off or on. We could add an environment variable to control this
> > feature. But I'd really like to hear the compelling use case first
> > (i.e., both why it cannot simply be spelled "git config -z --list", and
> > why not following includes is not OK).
>
> Hopefully my explanation conveys why "git config -z --list" is insufficient.
Yes, I agree it's insufficient in the face of includes. Unfortunately, I
don't think simply expanding includes by default is sufficient, either.
Probably the simplest solution for this case would be something like:
1. Always expand includes on listing.
2. When editing, always override items in includes. So do _not_ reuse
sections, but always put the values at the end of the file (or at
least after any includes).
That's not as nice as doing it "right" (you'll end up with duplicated
values in included files and their parents, and sections may appear
multiple times). But it's simple and should have the desired semantics.
> From a naive user's POV --
> I am asking "git config" to evaluate this file. I don't care what's
> inside of it; it's a black box to me. All I know is that I get config
> values out the other side. Sometimes git respects the included files.
> Sometimes it does not (when using -f).
> ^^^^^^^^^
> I think this subtle difference in behavior is best avoided.
It is not "sometimes when I ask git config to evaluate this file". It is
"git never expands includes when you ask it to evaluate a file".
Includes are expanded only when you ask git "search all files you know
about" (i.e., when you do not provide a file argument).
There is also a more complex issue with "-f", which is that git-config
is used not just to parse git configuration, but also to parse other
config-like files that should not respect includes (e.g.,
".gitmodules"). So to remain backwards-compatible, we need the default
for "-f" to not respect includes, and that is not really negotiable.
We could convert "--global" and "--system" to respect includes by
default (since we at least know these are actual git config files). But
that is introducing a subtle difference, which you complained about
above.
But if scripted uses like "git cola" are the ones that need it, I'd
rather provide a backwards-compatible way of tweaking the behavior (if a
command line option is not sufficient, then I'd rather provide an
environment variable).
> Here's another. The naive user will do: `git config --list -f file`.
> They will then edit that file to add an include statement. They will
> then run `git config --list -f file` again and be confused as to why
> their included configuration is not honored.
As I mentioned above, that one is not really negotiable. But it seems
like it is a mismatch of expectations to behavior, and that is something
we can at least address with documentation.
> Right now these two are synonymous: "git config --system --list" and
> "git config -f $(prefix)/etc/gitconfig --list". Having one form honor
> includes and not the other is inconsistent.
With my patch, they are consistent, and neither should honor includes
(that being said, there is a bug in my patch which makes --list honor
includes in all cases; however, you are responding correctly to the
intended and advertised behavior of my patch, and that is what we should
be discussing).
> What if we frame this the other way around -- when would we not want
> it to follow includes? I can imagine someone writing a "git config"
> editing program, but I think that use case is rare (and they haven't
> spoken up, either ;-).
But isn't "git cola" a git-config editing program?
> Well.. okay. My use case is rare too! The dumb "cache the values
> behind a stat() call" thing has worked really well in practice so I've
> been happy with it so far. It's dumb, and it won't notice when
> included files change, but I really don't care because it solves the
> 99% case.
I don't get the caching. How often would you need to call "git config"?
Isn't is only when the edit-config dialog appears? Is it really that
expensive to call when opening a user-editable dialog?
> Rewriting it (for me) would mean using --global and --system and
> probably not caring so much about the distinction of repo vs. user
> configuration (or rather, deal with the fact that `git config --list`
> returns both the user config and the repo config). At that point I
> might as well just write (reuse) a proper .gitconfig parser, but
> what's the fun in that when "git config" is so nice and easy to parse?
I definitely feel your pain. I don't want you to have to rewrite yet
another .gitconfig parser. Let's see if we can figure out reasonable
editing semantics that can go into git-config.
-Peff
^ permalink raw reply
* Re: [PATCH 3/6] Stop producing index version 2
From: Thomas Rast @ 2012-02-07 17:25 UTC (permalink / raw)
To: Shawn Pearce
Cc: Junio C Hamano, Nguyễn Thái Ngọc, git,
Joshua Redstone
In-Reply-To: <CAJo=hJvtRnmvALcn3vKpYTr3j6ada8iboPjWN3cQnwwKzRvrDA@mail.gmail.com>
Shawn Pearce <spearce@spearce.org> writes:
> I have long wanted to scrap the current index format. I unfortunately
> don't have the time to do it myself. But I suspect there may be a lot
> of gains by making the index format match the canonical tree format
> better by keeping the tree structure within a single file stream,
> nesting entries below their parent directory, and keeping tree SHA-1
> data along with the directory entry.
If I may add to this: the one thing that I would like to see fixed about
the index is that it's flat out impossible to change a single thing in
it without re"writing" it from scratch.
I'm saying "writing" because it is possible to change a few things
around, but recomputing the trailing SHA1 swamps that by a large margin
unless you are writing to a floppy disk, so it doesn't matter. I'm sure
using a CRC32 helps here, but if we're going to make an incompatible
change, why not go all the way?
A tree layout can fix that if it is properly arranged so that if you
'git add path/to/file', it only updates the SHA1s for path/to/file,
path/to and path. For this to work, the checks would have to correspond
to the trees, perhaps even directly use the actual tree SHA1. This
would at least be natural in some sense; getting to actual log(n)
complexity for hilariously large directories would require dynamically
splitting directories where appropriate.
Along the same lines the format should allow for changing the extension
data for a single extension while only rehashing the new data.
When I worked on cache-tree, I considered making a change to the latter
effect, but thought the impact too great for a little gain. Now from
this thread, I'm getting the impression that such a change would be ok,
even if users would have to scrap the index if they downgrade. Is that
right?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: cloning a tree which has detached branch checked out
From: Jeff King @ 2012-02-07 17:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyen Thai Ngoc Duy, Michael S. Tsirkin, Jakub Narebski, git
In-Reply-To: <7v4nv236xn.fsf@alter.siamese.dyndns.org>
On Tue, Feb 07, 2012 at 09:11:00AM -0800, Junio C Hamano wrote:
> > This particular bug should have been fixed before that, even, with my
> > c1921c1 (clone: always fetch remote HEAD, 2011-06-07). And it is tested
> > explicitly in t5707,...
> [...]
> What is funny is "error: Trying to write ref HEAD with nonexistant object".
> "git grep -e nonexistant f3fb0" does not register any hit.
That misspelling of "nonexistent" was fixed by 7be8b3b (Fix typo:
existant->existent, 2011-06-16), around the same time as my clone patch.
Which really makes me wonder if the OP is accidentally running an old
version of git during the tests.
> >> On Tue, Feb 7, 2012 at 5:41 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> > #git clone -n lab:/home/mst/scm/linux
> [...]
> Could it be that mysterious "lab:" protocol handler that is misbehaving?
I think you are misreading. It would be "lab::" in that case, no? The
command above should do ssh transport to the machine "lab".
-Peff
^ permalink raw reply
* Re: cloning a tree which has detached branch checked out
From: Junio C Hamano @ 2012-02-07 17:11 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, Michael S. Tsirkin, Jakub Narebski, git
In-Reply-To: <20120207153225.GA14773@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Feb 07, 2012 at 07:57:08PM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> On Tue, Feb 7, 2012 at 5:41 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > #git clone -n lab:/home/mst/scm/linux
>> >> > ....
>> >> > error: Trying to write ref HEAD with nonexistant object
>> >> > cec64082f689f949a397cb9b39423dc41545fa0e
>> >> > fatal: Cannot update the ref 'HEAD'.
>> >> >
>> >> > Looks like a bug, doesn't it?
>> >> ...
>
> This particular bug should have been fixed before that, even, with my
> c1921c1 (clone: always fetch remote HEAD, 2011-06-07). And it is tested
> explicitly in t5707,...
That was my thought when I first saw it. The failure case was that HEAD
was pointing at something no ref reaches, and clone was only grabbing the
tips of branches, which failed to transfer the history leading to HEAD.
> ... So I think your guess about a subtle error in the local ref
> processing is a reasonable one.
What is funny is "error: Trying to write ref HEAD with nonexistant object".
"git grep -e nonexistant f3fb0" does not register any hit.
Could it be that mysterious "lab:" protocol handler that is misbehaving?
^ permalink raw reply
* Re: [PATCH] gitweb: Deal with HEAD pointing to unborn branch in "heads" view
From: Jakub Narebski @ 2012-02-07 16:53 UTC (permalink / raw)
To: rajesh boyapati; +Cc: git
In-Reply-To: <CA+EqV8w6k2VrEtMydhGKZHbQdXHxCE3WA_0rtS-AY4cmQvii=A@mail.gmail.com>
On Mon, 6 Feb 2012, rajesh boyapati wrote:
>
> Thanks for your work.
I'm sorry I was able to find a fix only for the part of issue.
>>>>>>>>> [2012-01-25 18:50:35,658] ERROR
>>>>>>>>> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: [Wed Jan 25
>>>>>>>>> 18:50:35 2012] gitweb.cgi: Use of uninitialized value $commit_id
>>>>>>>>> in open at /usr/lib/cgi-bin/gitweb.cgi line 2817.
>>>>>>>
>>>>>>> sub parse_commits {
>>>>>>> my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
>>>>>>> my @cos;
>>>>>>>
>>>>>>> $maxcount ||= 1;
>>>>>>> $skip ||= 0;
>>>>>>>
>>>>>>> local $/ = "\0";
>>>>>>>
>>>>>>> open my $fd, "-|", git_cmd(), "rev-list",
>>>>>>> "--header",
>>>>>>> @args,
>>>>>>> ("--max-count=" . $maxcount),
>>>>>>> ("--skip=" . $skip),
>>>>>>> @extra_options,
>>>>>>> $commit_id,
>>>>>>> "--",
>>>>>>> ($filename ? ($filename) : ())
>>>>>>> or die_error(500, "Open git-rev-list failed");
>>
>> But I was not able to fix this, at least not currently. I wrote a failing
>> test case for "commit" and similar views on unborn HEAD... but they fail
>> _without_ error message like the one quoted.
>>
>> I'd have to go slower route of examining gitweb code in how it deals with
>> "invalid" HEAD (i.e. HEAD not pointing to commit, being on unborn branch).
[...]
>> And here is the patch:
>> -->8 ------------>8 ---
>> From: Jakub Narebski <jnareb@gmail.com>
>> Subject: [PATCH] gitweb: Deal with HEAD pointing to unborn branch in
>> "heads" view
[...]
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 9cf7e71..1f0ec12 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -5570,7 +5570,7 @@ sub git_tags_body {
>>
>> sub git_heads_body {
>> # uses global variable $project
>> - my ($headlist, $head, $from, $to, $extra) = @_;
>> + my ($headlist, $head_at, $from, $to, $extra) = @_;
>> $from = 0 unless defined $from;
>> $to = $#{$headlist} if (!defined $to || $#{$headlist} < $to);
>>
>
> I didn't see a file called "gitweb.perl" in /usr/share/gitweb/
The file "gitweb.perl", or rather "gitweb/gitweb.perl" is the name
of the script in git.git repository. From it "make gitweb" would
generate "gitweb.cgi" file...
> I applied this patch to file "index.cgi" in /usr/share/gitweb/index.cgi at
> line 4711.
[...]
>
> I applied this patch to file "index.cgi" in /usr/share/gitweb/index.cgi at
> line 4720.
...and I guess Gerrit build process generates "index.cgi" from that.
> Had I applied the patch to the correct file "index.cgi", which is a link to
> file "gitweb.cgi" in /usr/lib/cgi-bin/gitweb.cgi ?
Ah, right.
> Then, I restarted gerrit server to take changes.
> Now the error log of gerrit shows:
> [2012-02-06 11:21:46,726] ERROR
> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision
> 'HEAD'
> [2012-02-06 11:21:49,167] ERROR
> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: [Mon Feb 6 11:21:49
> 2012] gitweb.cgi: Use of uninitialized value $commit_id in open at
> /usr/lib/cgi-bin/gitweb.cgi line 2817.
> [2012-02-06 11:21:49,169] ERROR
> com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision ''
[the same errors repeated few times]
> <<<<<<<<<<<<<<<<
> Previously, there is a error showing at line 4720. Now, with this patch,
> that error has gone.
As I said I was able to find a fix only for part of the issue.
Unfortunately I was not able to reproduce this error in this form.
Note that the error location doesn't help much, because it is more
interesting for find which callers of parse_commits() pass undefined
$commit_id.
I can try to harden parse_commits() against bogus parameters; maybe
this would help.
> I tried to upgrade gitweb with the command "sudo apt-get install gitweb",
> but, it didn't find any upgrade.
> Am I doing in a right way?
There is no new version of gitweb yet; it haven't even been accepted
by Junio Hamano, maintainer of git of which gitweb is part, into git
repository (I might have to resend this patch for better visibility).
> Is there any place like "Github" (where we can place git projects) for
> gitweb ?
Gitweb is for quite some time developed within git repository. From
it the 'gitweb' package is created.
Clones of canonical, official git repository can be found in a few places:
git://git.kernel.org/pub/scm/git/git.git
git://repo.or.cz/alt-git.git
https://code.google.com/p/git-core/
https://github.com/git/git
My own clone of git, with my work, can be found at:
git://repo.or.cz/git/jnareb-git.git
https://github.com/jnareb/git
>> diff --git a/t/t9500-gitweb-standalone-no-errors.sh
>> b/t/t9500-gitweb-standalone-no-errors.sh
>> index 0f771c6..81246a6 100755
>> --- a/t/t9500-gitweb-standalone-no-errors.sh
>> +++ b/t/t9500-gitweb-standalone-no-errors.sh
>> @@ -739,4 +739,13 @@ test_expect_success \
>> 'echo "\$projects_list_group_categories = 1;">>gitweb_config.perl
>> &&
>> gitweb_run'
>>
>> +# ----------------------------------------------------------------------
>> +# unborn branches
>> +
>> +test_expect_success \
>> + 'unborn HEAD: "summary" page (with "heads" subview)' \
>> + 'git checkout orphan_branch || git checkout --orphan orphan_branch
>> &&
>> + test_when_finished "git checkout master" &&
>> + gitweb_run "p=.git;a=summary"'
>> +
>> test_done
>>
>
> I didn't find a file where to apply this patch.
> Is this file to test your patch for you?
Yes, this is to test that my patch fixes the issue correctly, and to
ensure that further changes don't re-break it. It is not usually
installed with git or gitweb, so don't worry about it.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v2 0/4] Deprecate "not allow as-is commit with i-t-a entries"
From: Junio C Hamano @ 2012-02-07 16:41 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Jonathan Nieder
In-Reply-To: <1328618804-31796-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> - git-add.txt changes are removed. In the end all kinds of commit
> behave the same way, not worth putting more explanation during the
> transition.
>
> - reword config text and warning text (or more precisely copy/paste
> from Junio/Jonathan's words)
>
> - Hard coded release numbers are removed. Now it's simply "in future".
>
> - Step 2 may be too annoying. Users are warned on every commit if
> commit.ignoreIntentToAdd is set. I think it's good because it keeps
> config file clean, but people may think otherwise.
Ahh, thanks.
But when I said "let's admit that this is just fixing an UI mistake, no
configuration, no options", I really meant it. Without the backward
compatiblity "For now please do not fix this bug for me and keep being
buggy until I get used to the non-buggy behaviour" fuss, which we never do
to any bugfix.
That is how we are planning to handle "git merge" update to spawn editor
in interactive session in the next release. There is no "Please keep the
buggy behaviour" option; only an environment variable to help when we
mistake a scripted use as interactive, whose support is not going away
because it is not about "until I get used to the new behaviour".
^ permalink raw reply
* Re: User authentication in GIT
From: supadhyay @ 2012-02-07 16:40 UTC (permalink / raw)
To: git
In-Reply-To: <CALKQrgdvOhfhTPg+g+LqCb6XOQczcz-nYC61B9x4W5dB4Up5oA@mail.gmail.com>
Thank you Johan,freak.
you have clear my doubts at some extent and I think let me work on it.
I will work on giloite and get back to you.
Thanks...
--
View this message in context: http://git.661346.n2.nabble.com/User-authentication-in-GIT-tp7261349p7262934.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCHv2] tag: add --points-at list option
From: Jeff King @ 2012-02-07 16:05 UTC (permalink / raw)
To: Tom Grennan; +Cc: git, gitster, jasampler
In-Reply-To: <1328598076-7773-2-git-send-email-tmgrennan@gmail.com>
On Mon, Feb 06, 2012 at 11:01:16PM -0800, Tom Grennan wrote:
> +struct points_at {
> + struct points_at *next;
> + unsigned char *sha1;
> +};
Would using sha1_array save us from having to create our own data
structure? As a bonus, it can do O(lg n) lookups, though I seriously
doubt anyone will provide a large number of "--points-at".
> +static void free_points_at (struct points_at *points_at)
> +{
> + while (points_at) {
> + struct points_at *next = points_at->next;
> + free(points_at->sha1);
> + free(points_at);
> + points_at = next;
> + }
> +}
Then this could go away in favor of sha1_array_clear.
> +int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
> +{
> + struct points_at *new, **opt_value = (struct points_at **)opt->value;
> + unsigned char *sha1;
> +
> + if (!arg)
> + return error(_("missing <object>"));
> + new = xmalloc(sizeof(struct points_at));
> + sha1 = xmalloc(20);
> + if (get_sha1(arg, sha1)) {
> + free(new);
> + free(sha1);
> + return error(_("malformed object name '%s'"), arg);
> + }
> + new->sha1 = sha1;
> + new->next = *opt_value;
> + *opt_value = new;
> + return 0;
> +}
And this can drop all of the memory management bits, like:
unsigned char sha1[20];
if (!arg)
return error(_("missing <object>"));
if (get_sha1(arg, sha1))
return error(_("malformed object name '%s'"), arg);
sha1_array_append(opt->value, sha1);
return 0;
Also, should you check "unset"? When we have options that build a list,
usually doing "--no-foo" will clear the list. E.g., this:
git tag --points-at=foo --points-at=bar --no-points-at --points-at=baz
should look only for "baz".
> +static struct points_at *match_points_at(struct points_at *points_at,
> + const unsigned char *sha1)
> +{
> + char *buf;
> + struct tag *tag;
> + unsigned long size;
> + enum object_type type;
> +
> + buf = read_sha1_file(sha1, &type, &size);
> + if (!buf)
> + return NULL;
> + if (type != OBJ_TAG
> + || (tag = lookup_tag(sha1), !tag)
> + || parse_tag_buffer(tag, buf, size) < 0) {
> + free(buf);
> + return NULL;
> + }
> + while (points_at && hashcmp(points_at->sha1, tag->tagged->sha1))
> + points_at = points_at->next;
> + free(buf);
> + return points_at;
> +}
Sorry, I threw a lot of object lookup code at you last time, so I think
my point may have been lost in the noise. But I think this is slightly
nicer as:
static int tag_points_at(struct sha1_array *sa,
const unsigned char *sha1)
{
struct object *obj = parse_object(sha1);
if (!obj)
return 0; /* or probably we should even just die() */
if (obj->type != OBJ_TAG)
return 0;
if (sha1_array_lookup(sa, ((struct tag *)obj)->tagged->sha1) < 0)
return 0;
return 1;
}
I.e., using parse_object lets you avoid dealing with memory management
yourself. And as a bonus, it will reuse the cached information if you
happen to have already parsed that object (not likely in typical
repositories, but a huge win in certain pathological cases, like repos
storing shared objects and refs for a large number of forks).
> + {
> + OPTION_CALLBACK, 0, "points-at", &points_at, "object",
> + "print only annotated|signed tags of the object",
> + PARSE_OPT_LASTARG_DEFAULT,
> + parse_opt_points_at, (intptr_t)NULL,
> + },
I think you can drop the LASTARG_DEFAULT here, as it is no longer
optional, no?
-Peff
^ permalink raw reply
* Re: Merging only a subdirectory from one branch to the other
From: Taylor Hedberg @ 2012-02-07 15:24 UTC (permalink / raw)
To: Howard Miller; +Cc: git
In-Reply-To: <CAHVO_90MQamw7oB8ry5YBEWSnRnxDZvQ4ApVuuv4AYR6VRuXSw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 790 bytes --]
Howard Miller, Tue 2012-02-07 @ 09:38:22+0000:
> I have a branch with a particular subdirectory tree. The tree has a
> lot of history. However, all the history for that subdirectory is
> exclusive to it (no commits changed anything outside it). I now need
> to merge that subdirectory into a completely different branch without
> loosing any history. I think git-read-tree might have something to do
> with it but I don't understand the help file at all. Any help
> appreciated.
You might find git-subtree [1] useful for this. The `git subtree split`
sub-command in particular enables you to factor out the history of a
particular subdirectory in your repo into a separate branch, which you
could then merge into some other branch as desired.
[1] https://github.com/apenwarr/git-subtree
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: cloning a tree which has detached branch checked out
From: Jeff King @ 2012-02-07 15:32 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Michael S. Tsirkin, Jakub Narebski, git
In-Reply-To: <CACsJy8DtmQLX+Lfng-QRzVg9sfo8gQMXB-xTtPYpt+R2gModTg@mail.gmail.com>
On Tue, Feb 07, 2012 at 07:57:08PM +0700, Nguyen Thai Ngoc Duy wrote:
> On Tue, Feb 7, 2012 at 5:41 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > #git clone -n lab:/home/mst/scm/linux
> >> > ....
> >> > error: Trying to write ref HEAD with nonexistant object
> >> > cec64082f689f949a397cb9b39423dc41545fa0e
> >> > fatal: Cannot update the ref 'HEAD'.
> >> >
> >> > Looks like a bug, doesn't it?
> >>
> >> Which git version? IIRC there was some bugfix recently in that
> >> area...
> >
> > Sorry, forgot to mention that. It's pretty recent:
> > $ git --version
> > git version 1.7.9.111.gf3fb0
>
> The series that Jakub mentioned is probably nd/clone-detached 5ce2b97,
> which is already included in your tree. Does 1.7.9 work?
>
> I tried but it was ok for me. I think ref processing at local probably
> goes wrong and does not request commit from HEAD. If the repo is not
> confidential, you can zip it and send me.
This particular bug should have been fixed before that, even, with my
c1921c1 (clone: always fetch remote HEAD, 2011-06-07). And it is tested
explicitly in t5707, so your series does not regress at least a simple
case. So I think your guess about a subtle error in the local ref
processing is a reasonable one.
-Peff
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Johan Herland @ 2012-02-07 15:09 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Philip Oakley, git
In-Reply-To: <201202071531.08385.jnareb@gmail.com>
(we are pretty much in violent agreement, so I will only comment where
I find it necessary)
On Tue, Feb 7, 2012 at 15:31, Jakub Narebski <jnareb@gmail.com> wrote:
> Also, when thinking about different scenarios of why one would like to
> mark commit as 'secret', we might want to be able to mark commit as
> secret / unpublishable with respect to _subset_ of remotes, so e.g.
> I am prevented from accidentally publishing commits marked as 'secret'
> to public repository, or to CI/QA repository, but I can push (perhaps
> with warning) to group repository, together with 'secret'-ness state
> of said commit...
>
> ... though it wouldn't be as much 'secret' as 'confidential' ;-)
Another way to achieve this would be to have a config flag to control
whether Git checks for the 'secret' flag before pushing. This config
flag could be set at the system/user level (to enable/disable the
feature as a whole), at the repo level (to enable/disable it in a
given repo), at the remote level (to enable/disable it on a given
repo), and finally at the branch level (to enable-disable it for a
given branch (and its upstream)). Thus you could have a .git/config
that looked like this:
[core]
refusePushSecret = true
[remote "foo"]
refusePushSecret = false
url = ...
fetch = ...
[branch "baz"]
remote = foo
merge = refs/heads/baz
refusePushSecret = true
This config would:
- refuse to push 'secret' commits from branch 'baz'
(branch.baz.refusePushSecret == true)
- but allow to push other branches with 'secret' commits to remote
'foo' (remote.foo.refusePushSecret == false)
- but refuse to push 'secret' commits to other remotes
(core.refusePushSecret == true)
(The order of precedence would be: branch config > remote config >
repo config > user config > system config > default when unset)
I am unsure whether the 'secret'-ness of a commit should follow across
the push, but if you do (assuming we store the 'secret' flag using
git-notes) this is simply a matter of synchronizing the
refs/notes/secret to the same remote.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [RFD] Rewriting safety - warn before/when rewriting published history
From: Jakub Narebski @ 2012-02-07 14:31 UTC (permalink / raw)
To: Johan Herland; +Cc: Philip Oakley, git
In-Reply-To: <CALKQrgcUdigB5zB_bqgpW8=o-TuGChs+q2nYoXu5YdyWu+oWZw@mail.gmail.com>
On Mon, 6 Feb 2012, Johan Herland wrote:
> On Mon, Feb 6, 2012 at 18:14, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Mon, 6 Feb 2012, Johan Herland wrote:
>>> On Mon, Feb 6, 2012 at 15:44, Jakub Narebski <jnareb@gmail.com> wrote:
[...]
>> Yes, starting with prototype implementation using existing infrastructure
>> (hooks) would be a very good idea. (That's how first versions of what
>> became submodules were implemented.)
>>
>> OTOH we should be aware of limitations of said prototype due to the fact
>> that it is a prototype...
>
> Agreed, but AFAICS (and modulo the addition of pre-rewrite and
> pre/post-push hooks mentioned earlier) all of the things discussed so
> far in this thread can be implemented as hooks.
That would be nice.
And the new hooks (pre-rewrite, pre/post-push) would be useful anyway,
I think.
[...]
>>> And trying to plug too many holes might end
>>> up annoying more experienced users who "know what they're doing".
>>
>> Second, forcing via command line parameter should always be an option.
>
> Obviously, but if a large portion of the Git community felt the need
> to always disable this feature, I'd say that we'd failed. The best
> features are those that Just Work(tm).
Well, we have some features in git like advice.* that cater to newbies.
This could be such newbie-mostly feature too.
Anyway what I was thinking about is that forcing is for those hopefully
rare cases where Git safety net is too strict, where DWIM-mery fails.
>>> Instead we might want to add a client-side check at push time. I
>>> realize that this check is already done by the remote end, but the
>>> client-side might be able to give a more helpful response along the
>>> lines of:
>>
>> [...]
>>
>> Explanation is good, but the whole idea of rewriting safety is that you
>> are informed (warned or denied) _before_ attempting rewrite and doing much
>> work.
>
> True, but there may be cases where the rewrite is not apparent until
> after it has happened. E.g. a novice user may use 'git reset --hard'
> in order to get to an earlier state for testing purposes, and then -
> after completing the test - 'git reset --hard' back to the starting
> point. I know this is not best practice, but is it bad enough that we
> want to refuse it?
I'd like to have safety net also for using 'git reset --hard' to rewind
publishable branch past published history. Though preventing e.g. force
rename of other branch to publishable branch so it doesn't fast-forward
WRT its upstream would be going to far, I guess...
>>> First, we don't need to annotate _all_ commits. For the 'public'
>>> state, we only annotate the last/tip commit that was pushed/published.
>>> From there, we can defer that all ancestor commits are also 'public'.
>>
>> Right.
>>
>>> For the 'secret' state, we do indeed annotate _all_ secret commits,
>>> but I believe this will be a somewhat limited number of commits. If
>>> your workflow forces you to annotate millions of commits as 'secret',
>>> I claim there is something wrong with your workflow.
>>
>> Well, for the 'secret' we can rely on the fact that child of 'secret'
>> commit must also be 'secret' (non-publishable) if secret is to stay
>> secret. Still marking all 'secret' commits might be better idea from
>> UI and from performance point of view.
>
> I don't think we should automatically assume that all children of a
> 'secret' commit are also 'secret'.
All right. What is important is that safety net related to 'secret'
commits would prevent publishing such commits even if you build non-secret
commits on top of 'secret' one.
> First of all, the git DAG was not
> made for iterating forwards in history, so given a 'secret' commit, it
> is computationally expensive to enumerate all its implied-'secret'
> descendants.
Right, this would enormously complicate detecting 'secret'-ness of commit
when browsing history / querying "phase" of a commit..
> More importantly though, I don't agree with the premise.
> I would typically use the 'secret' state as follows: While debugging a
> piece of code, I might commit a few debug print statements, and I
> would typically mark this debug commit as 'secret', in order to
> prevent myself from accidentally pushing this. Although it probably
> doesn't matter in practice, I think it is wrong for the commits made
> (temporarily) on top of this debug commit to be also considered
> 'secret'. They are only unpublishable as a consequence of being based
> on the debug commit, and only until I get around to rebasing away the
> 'secret' debug commit.
Right, thanks for sanity check.
So another difference that in its full complexity the 'secret' is an
attribute of an individual commit which is (usually) local to repository,
while 'public' is in its full complexity an attribute of revision walking
rather than something that can be attached to a commit.
Also, when thinking about different scenarios of why one would like to
mark commit as 'secret', we might want to be able to mark commit as
secret / unpublishable with respect to _subset_ of remotes, so e.g.
I am prevented from accidentally publishing commits marked as 'secret'
to public repository, or to CI/QA repository, but I can push (perhaps
with warning) to group repository, together with 'secret'-ness state
of said commit...
... though it wouldn't be as much 'secret' as 'confidential' ;-)
>>>> Second, I have doubts if "phase" is really state of an individual commit,
>>>> and not the feature of revision walking.
>>
>> It matters to presentation: can commit be simultaneously 'public' because
>> of one branch, and 'draft' because of other.
>>
>>> I believe the 'public' state is a "feature of revision walking" (i.e.
>>> one annotated 'public' commit implies that all its ancestors are also
>>> 'public'). However, the 'secret' state should be bound to the
>>> individual commit, IMHO.
>>
>> Good call, otherwise 'secret' commit could have been "side-leaked"
>> by other refs being pushed.
>>
>> This means though that 'public' / 'draft' while looking similar to 'secret'
>> are in fact a bit different things. In other words 'immutable' and
>> 'impushable' traits are quite a bit different in behavior...
>>
>> Especially that one acts at pre-rewrite time, and second pre-push time.
>
> Exactly. I find Mercurial 'phase' language confusing, precisely for
> the reason that 'public' and 'secret' are DIFFERENT concepts. One
> hinders rewrite and naturally applies to a commit AND its ancestors,
> while the other hinders push and only applies to the commit itself.
> The fact that they could be implemented by the same mechanisms (hooks
> and notes) does not make them the same thing.
Well, if one doesn't think about all possible usages and complications,
the simplicity of metaphor of "phases" is quite compelling. We have
(at first glance)
public < draft < secret
states of a commit (changeset), just like we have
solid < liquid < gaseous
phases of matter.
If we treat traits of 'immutability' and 'publishability' as true boolean,
such ordering of "phases" looks natural. Just like matter goes from gas
to liquid to solid with lowering temperature, changeset "phase" goes from
'secret' to 'draft' to 'public' following ancestry chain.
But if we go for expressivity and power like the rest of Git, rather than
for simplicity like Mercurial does, the differences between 'secret'-ness
and 'public'-ness make it unlikely that it would be a good idea to encompass
those in a single UI and a single concept. I guess that it can be another
issue where Mercurial oversimplifies its approach (c.f. branches which
started from clone to branch and anonymous branches, to "named branches"
which are really commit labels, to local-only bookmark branches, to
transferrable Git-like bookmark branches but with single namespace, and
only just considering equivalent of Git's refspec and refs mapping; c.f.
revision ranges based on local numbering of commits which made ranges
local too, to new Git-like topological `--ancestry-path`-like ranges).
Also with per-remote 'secret'-ness, we might have the scenario where
a commit is marked as 'secret' for public repo, we have pushed it to
group repo (and it's 'secret'-ness trait together with it), an now we
want to be warned if we try to rewrite said commit.
So, separate traits it is (of revision walk, or of a commit), rather
than "smush-together" antity / attribute of "phase" of revision.
That is BTW why I wanted us to have this discussion... :-D
[...]
>>> Basically, there are three different "levels" for this rewrite/publish
>>> protection to run at:
>>>
>>> 1. Do not meddle at all. This is the current behavior, and assumes
>>> that if the user rewrites and pushes something, the user knows what
>>> he/she is doing, and Git should not meddle (obviously unless the
>>> server refuses the push).
>>
>> I think that there should be some easy way to force such behavior,
>> i.e. to discard rewrite safeties.
>
> Indeed. We should probably have a simple config flag to enable the
> rewrite protection. In fact I would argue that the flag should default
> to false (disable protection) when unset, and then we should let
> init/clone set the config flag to true (enable protection) in newly
> created repos (unless explicitly disabled in the user/system configs).
> This way, behavior does not change for existing repos, but new repos
> are protected by default (with only a single command needed to disable
> the protection).
And commit option for discarding safeties for specific push, or for
specific rewrite (e.g. pushed at night, noticed an error in commit
message, force-amended, and force-pushed again, hoping that the window
of opportunity is sufficintly small so that nobody will be affected).
>>> 2. Warn/refuse rewriting commits in your upstream. This would only
>>> check branch X against its registered upstream. Only if there is a
>>> registered upstream, and you're about to rewrite commits that are
>>> reachable from the upstream remote-tracking branch, should Git
>>> intervene and warn/refuse the rewrite. This level would IMHO provide
>>> most of the benefit, and little or no trouble (i.e. false positives).
>>
>> Right. I wonder if we can get usage statistics from Mercurial users
>> about usage of their "phases" feature... though mapping terminology
>> for example 'upstream' from Git to Mercurial and vice versa can be
>> a pain, I guess.
>
> I'm unsure how useful it would be. IMHO Git and Mercurial are
> different enough (and promote sightly different workflows) that I
> don't trust the the average Mercurial user's preference for
> Mercurial's 'phase' behavior to be transferable to the average Git
> user's preference for a similar behavior in Git.
Well, bad statistics might be better than no statistics. What would
be especially interesting is _complaints_ about "phases" feature,
don't you think, i.e. where the "phases" metaphor fails.
[...]
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Git performance results on a large repository
From: Nguyen Thai Ngoc Duy @ 2012-02-07 13:43 UTC (permalink / raw)
To: Joey Hess; +Cc: git@vger.kernel.org
In-Reply-To: <20120206154043.GA14632@gnu.kitenet.net>
On Mon, Feb 6, 2012 at 10:40 PM, Joey Hess <joey@kitenet.net> wrote:
>> Someone on HN suggested making assume-unchanged files read-only to
>> avoid 90% accidentally changing a file without telling git. When
>> assume-unchanged bit is cleared, the file is made read-write again.
>
> That made me think about using assume-unchanged with git-annex since it
> already has read-only files.
>
> But, here's what seems a misfeature...
because, well.. assume-unchanged was designed to avoid stat() and
nothing else. We are basing a new feature on top of it.
> If an assume-unstaged file has
> modifications and I git add it, nothing happens. To stage a change, I
> have to explicitly git update-index --no-assume-unchanged and only then
> git add, and then I need to remember to reset the assume-unstaged bit
> when I'm done working on that file for now. Compare with running git mv
> on the same file, which does stage the move despite assume-unstaged. (So
> does git rm.)
This is normal in the lock-based "checkout/edit/checkin" model. mv/rm
operates on directory content, which is not "locked - no edit allowed"
(in our case --assume-unchanged) in git. But lock-based model does not
map really well to git anyway. It does not have the index (which may
make things more complicated). Also at index level, git does not
really understand directories.
I think we could add a protection layer to index, where any changes
(including removal) to an index entry are only allowed if the entry is
"unlocked" (i.e no assume-unchanged bit). Locked entries are read-only
and have assume-unchanged bit set. "git (un)lock" are introduced as
new UI. Does that make assume-unchanged friendlier?
--
Duy
^ permalink raw reply
* STGIT: Deathpatch in linus tree
From: "Andy Green (林安廸)" @ 2012-02-07 13:02 UTC (permalink / raw)
To: git
Hi -
Linus just pushed 105e5180936d69b1aee46ead8a5fc6c68f4d5f65 to
linux-2.6... along the lines of the Monty Python joke that was so funny
it kills anyone who hears it, if I have a stgit branch based on a HEAD
that includes this commit then stgit dies when pushing on top of it.
So...
[agreen@build linux-2.6]$ stg pop --all
[agreen@build linux-2.6]$ git reset --hard 96e02d1
HEAD is now at 96e02d1 exec: fix use-after-free bug in setup_new_exec()
[agreen@build linux-2.6]$ stg push
Pushing patch "subject-patch-1-3-arm-dt-add-p" ... done (empty)
Now at patch "subject-patch-1-3-arm-dt-add-p"
[agreen@build linux-2.6]$ stg pop
Popped subject-patch-1-3-arm-dt-add-p
No patch applied
So the commit just before the bad guy is happy. Then -->
[agreen@build linux-2.6]$ git reset --hard 105e518
HEAD is now at 105e518 Merge tag 'hwmon-fixes-for-3.3-rc3' of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging
[agreen@build linux-2.6]$ stg push
Error: Unhandled exception:
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/stgit/main.py", line 152, in _main
ret = command.func(parser, options, args)
File "/usr/lib/python2.7/site-packages/stgit/commands/push.py", line
68, in func
check_clean_iw = clean_iw)
File "/usr/lib/python2.7/site-packages/stgit/lib/transaction.py", line
95, in __init__
self.__current_tree = self.__stack.head.data.tree
File "/usr/lib/python2.7/site-packages/stgit/lib/git.py", line 426, in
data
self.__repository.cat_object(self.sha1))
File "/usr/lib/python2.7/site-packages/stgit/lib/git.py", line 408, in
parse
assert False
AssertionError
It also dies if I use Linus' current HEAD as the basis I am stgitting on
top of, which is one patch ahead of the deathpatch.
I'm using Fedora rawhide versions of git and stgit
git-1.7.9-1.fc17.x86_64
stgit-0.15-2.fc17.noarch
-Andy
^ permalink raw reply
* Re: cloning a tree which has detached branch checked out
From: Nguyen Thai Ngoc Duy @ 2012-02-07 13:07 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jakub Narebski, git
In-Reply-To: <20120207130204.GA7600@redhat.com>
On Tue, Feb 7, 2012 at 8:02 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> I tried but it was ok for me. I think ref processing at local probably
>> goes wrong and does not request commit from HEAD. If the repo is not
>> confidential, you can zip it and send me.
>
> Can't unfortunately :(
> Would some verbose logs help?
You can try cloning with GIT_TRACE_PACKET=1. bisect should also help.
--
Duy
^ permalink raw reply
* Re: cloning a tree which has detached branch checked out
From: Michael S. Tsirkin @ 2012-02-07 13:02 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Jakub Narebski, git
In-Reply-To: <CACsJy8DtmQLX+Lfng-QRzVg9sfo8gQMXB-xTtPYpt+R2gModTg@mail.gmail.com>
On Tue, Feb 07, 2012 at 07:57:08PM +0700, Nguyen Thai Ngoc Duy wrote:
> On Tue, Feb 7, 2012 at 5:41 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > #git clone -n lab:/home/mst/scm/linux
> >> > ....
> >> > error: Trying to write ref HEAD with nonexistant object
> >> > cec64082f689f949a397cb9b39423dc41545fa0e
> >> > fatal: Cannot update the ref 'HEAD'.
> >> >
> >> > Looks like a bug, doesn't it?
> >>
> >> Which git version? IIRC there was some bugfix recently in that
> >> area...
> >
> > Sorry, forgot to mention that. It's pretty recent:
> > $ git --version
> > git version 1.7.9.111.gf3fb0
>
> The series that Jakub mentioned is probably nd/clone-detached 5ce2b97,
> which is already included in your tree. Does 1.7.9 work?
I'll try that.
> I tried but it was ok for me. I think ref processing at local probably
> goes wrong and does not request commit from HEAD. If the repo is not
> confidential, you can zip it and send me.
Can't unfortunately :(
Would some verbose logs help?
> --
> Duy
^ permalink raw reply
* Re: cloning a tree which has detached branch checked out
From: Nguyen Thai Ngoc Duy @ 2012-02-07 12:57 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jakub Narebski, git
In-Reply-To: <20120207104100.GA24828@redhat.com>
On Tue, Feb 7, 2012 at 5:41 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > #git clone -n lab:/home/mst/scm/linux
>> > ....
>> > error: Trying to write ref HEAD with nonexistant object
>> > cec64082f689f949a397cb9b39423dc41545fa0e
>> > fatal: Cannot update the ref 'HEAD'.
>> >
>> > Looks like a bug, doesn't it?
>>
>> Which git version? IIRC there was some bugfix recently in that
>> area...
>
> Sorry, forgot to mention that. It's pretty recent:
> $ git --version
> git version 1.7.9.111.gf3fb0
The series that Jakub mentioned is probably nd/clone-detached 5ce2b97,
which is already included in your tree. Does 1.7.9 work?
I tried but it was ok for me. I think ref processing at local probably
goes wrong and does not request commit from HEAD. If the repo is not
confidential, you can zip it and send me.
--
Duy
^ permalink raw reply
* [PATCH v2 4/4] commit: remove commit.ignoreIntentToAdd, assume it's always true
From: Nguyễn Thái Ngọc Duy @ 2012-02-07 12:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328618804-31796-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 13 -------------
builtin/commit.c | 35 +----------------------------------
t/t2203-add-intent.sh | 4 ++--
3 files changed, 3 insertions(+), 49 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index fa56753..abeb82b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -831,19 +831,6 @@ commit.template::
"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
specified user's home directory.
-commit.ignoreIntentToAdd::
- When set to `false`, prevent `git commit` from creating a
- commit from an index that has entries that were added with
- `git add -N` but have not been updated with real contents, as
- the user may have forgotten to tell the final contents for
- these entries. Setting this to `true` makes `git commit`
- pretend as if these entries do not exist in the index.
-+
-The default for this variable is `true`. You are discouraged to set it
-to `false` to keep the old behaviour a bit longer because support
-setting this to `false` will be removed in future releases without
-warning.
-
credential.helper::
Specify an external helper to be called when a username or
password credential is needed; the helper may consult external
diff --git a/builtin/commit.c b/builtin/commit.c
index cd28081..491cae1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -118,8 +118,6 @@ static enum {
} status_format = STATUS_FORMAT_LONG;
static int status_show_branch;
-static int set_commit_ignoreintenttoadd;
-
static int opt_parse_m(const struct option *opt, const char *arg, int unset)
{
struct strbuf *buf = opt->value;
@@ -423,20 +421,6 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
if (!pathspec || !*pathspec) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
- if (!(cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)) {
- int i;
- for (i = 0; i < active_nr; i++)
- if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
- break;
- if (i < active_nr) {
- error(_("you intended to add \"%s\" but did not add it; not committing\n"
- "this behavior is deprecated, please set commit.ignoreIntentToAdd\n"
- "to true or remove the configuration variable. See the configuration\n"
- "variable documentation for more information."),
- active_cache[i]->name);
- exit(128); /* die() */
- }
- }
if (active_cache_changed) {
update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1355,13 +1339,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
include_status = git_config_bool(k, v);
return 0;
}
- if (!strcmp(k, "commit.ignoreintenttoadd")) {
- set_commit_ignoreintenttoadd = 1;
- if (git_config_bool(k, v))
- cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
- else
- cache_tree_flags &= ~WRITE_TREE_IGNORE_INTENT_TO_ADD;
- }
status = git_gpg_config(k, v, NULL);
if (status)
@@ -1425,8 +1402,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
git_config(git_commit_config, &s);
determine_whence(&s);
- if (!set_commit_ignoreintenttoadd)
- cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+ cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
if (get_sha1("HEAD", sha1))
current_head = NULL;
@@ -1587,14 +1563,5 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!quiet)
print_summary(prefix, sha1, !current_head);
- if (set_commit_ignoreintenttoadd) {
- if (cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)
- warning(_("commit.ignoreIntentToAdd = true is not needed anymore.\n"
- "Please remove it."));
- else
- warning(_("commit.ignoreIntentToAdd = false is deprecated.\n"
- "Please see the commit.ignoreIntentToAdd documentation for\n"
- "more information and remove the configuration variable."));
- }
return 0;
}
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 09b8bbf..6dbfb74 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -44,13 +44,13 @@ test_expect_success 'cannot commit with i-t-a entry' '
git commit -minitial
'
-test_expect_success 'can commit tree with i-t-a entry' '
+test_expect_success 'commit.ignoreIntentToAdd = false is ignored' '
git reset --hard HEAD^ &&
echo xyzzy >rezrov &&
echo frotz >nitfol &&
git add rezrov &&
git add -N nitfol &&
- git config commit.ignoreIntentToAdd true &&
+ git config commit.ignoreIntentToAdd false &&
git commit -m initial &&
git ls-tree -r HEAD >actual &&
cat >expected <<EOF &&
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v2 3/4] commit: turn commit.ignoreIntentToAdd to true by default
From: Nguyễn Thái Ngọc Duy @ 2012-02-07 12:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328618804-31796-1-git-send-email-pclouds@gmail.com>
This is step 2 from commit.ignoreIntentToAdd deprecation plan. To
recap:
2. A few releases after step 1 is out in the field, turn
commit.ignoreIntentToAdd default value to true (affecting mostly new
users).
Those who decided to stick to "false" from step 1 are warned the "false"
support will soon be gone and encouraged to move to "true" (or simply
remove the config variable).
Those who set the config to "true" is advised to remove it to keep
config file clean.
Those who encountered the safety check and did not bother to set this
config var is left out in the cold.
3. A few more releases after step 2, commit.ignoreIntentToAdd is
removed. There's no way to bring back the safety check.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 11 ++++-------
builtin/commit.c | 19 ++++++++++++++++---
t/t2203-add-intent.sh | 4 ++--
3 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 6839e44..fa56753 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -839,13 +839,10 @@ commit.ignoreIntentToAdd::
these entries. Setting this to `true` makes `git commit`
pretend as if these entries do not exist in the index.
+
-The default for this variable is `false`, but it will change to `true`
-in future releases of git. To ease the transition, you may want to set
-it to `true` now and get used to the new behaviour early, or you may
-want to set it to `false` to keep the old behaviour a bit longer. We
-however expect to support setting this to `false` (to keep the current
-behaviour) only for a limited time after the default is changed to
-`true`.
+The default for this variable is `true`. You are discouraged to set it
+to `false` to keep the old behaviour a bit longer because support
+setting this to `false` will be removed in future releases without
+warning.
credential.helper::
Specify an external helper to be called when a username or
diff --git a/builtin/commit.c b/builtin/commit.c
index da67653..cd28081 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -423,15 +423,16 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
if (!pathspec || !*pathspec) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
- if (!set_commit_ignoreintenttoadd) {
+ if (!(cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)) {
int i;
for (i = 0; i < active_nr; i++)
if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
break;
if (i < active_nr) {
error(_("you intended to add \"%s\" but did not add it; not committing\n"
- "hint: to commit all changes to tracked files, use \"commit -a\"\n"
- "hint: to commit anyway without adding, set commit.ignoreIntentToAdd to true"),
+ "this behavior is deprecated, please set commit.ignoreIntentToAdd\n"
+ "to true or remove the configuration variable. See the configuration\n"
+ "variable documentation for more information."),
active_cache[i]->name);
exit(128); /* die() */
}
@@ -1424,6 +1425,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
git_config(git_commit_config, &s);
determine_whence(&s);
+ if (!set_commit_ignoreintenttoadd)
+ cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+
if (get_sha1("HEAD", sha1))
current_head = NULL;
else {
@@ -1583,5 +1587,14 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (!quiet)
print_summary(prefix, sha1, !current_head);
+ if (set_commit_ignoreintenttoadd) {
+ if (cache_tree_flags & WRITE_TREE_IGNORE_INTENT_TO_ADD)
+ warning(_("commit.ignoreIntentToAdd = true is not needed anymore.\n"
+ "Please remove it."));
+ else
+ warning(_("commit.ignoreIntentToAdd = false is deprecated.\n"
+ "Please see the commit.ignoreIntentToAdd documentation for\n"
+ "more information and remove the configuration variable."));
+ }
return 0;
}
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 88a508e..09b8bbf 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,11 +41,11 @@ test_expect_success 'cannot commit with i-t-a entry' '
echo frotz >nitfol &&
git add rezrov &&
git add -N nitfol &&
- test_must_fail git commit -minitial
+ git commit -minitial
'
test_expect_success 'can commit tree with i-t-a entry' '
- git reset --hard &&
+ git reset --hard HEAD^ &&
echo xyzzy >rezrov &&
echo frotz >nitfol &&
git add rezrov &&
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v2 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries
From: Nguyễn Thái Ngọc Duy @ 2012-02-07 12:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328618804-31796-1-git-send-email-pclouds@gmail.com>
"git add -N" is introduced some time ago to help prevent users's
forgetting to add new files when committing. "git add -N" sets
CE_INTENT_TO_ADD bit to index, hence i-t-a entries from now on. For
safety reasons, as-is commits in the existence of i-t-a entries are
rejected ("I intend to add these files. If I type "git commit -m foo", it
may be a mistake if those files's contents are not added yet. Stop me in
that case").
However unless you are doing "commit -a" or "commit pathspec", you are
responsible for adding all contents you want to have in the commit
before you run the "git commit" command (and for the purpose of this
statement, "add -N" to tell Git to keey an eye on a path does _not_ add
contents).
A change to the file in the working tree that is left unadded is what
you decided to deliberately leave out of the commit, be it a change to a
path already in HEAD, or a path marked with "add -N". Forgetting to add
modified file and forgetting to add a file you earlier used "add -N"
amount to the same kind of risk, and "git status" is the way to make
sure your partial commit has exactly what you want. If you are not
worried about partial commit, you would be doing "commit -a", so the
"safety" is a moot point.
This patch is the beginning of the deprecation process to remove this
safety check, allowing "git commit" to proceed anyway. The process
consists of three steps to help users get used to new behavior:
1. Introduce a temporary config var, commit.ignoreIntentToAdd to help
the migration process, default to false. If the var is true, the remove
safety check.
Users who encounter the safety check are pointed to
commit.ignoreIntentToAdd documentation and advised to set the variable
to true.
2. A few releases after step 1 is out in the field, turn
commit.ignoreIntentToAdd default value to true (affecting mostly new
users).
Those who decided to stick to "false" from step 1 are warned the "false"
support will soon be gone and encouraged to move to "true" (or simply
remove the config variable).
Those who set the config to "true" is advised to remove it to keep
config file clean.
Those who encountered the safety check and did not bother to set this
config var is left out in the cold.
3. A few more releases after step 2, commit.ignoreIntentToAdd is
removed. There's no way to bring back the safety check.
This patch implements step 1.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/config.txt | 16 ++++++++++++++++
builtin/commit.c | 29 ++++++++++++++++++++++++++---
cache-tree.c | 8 +++++---
cache-tree.h | 1 +
t/t2203-add-intent.sh | 21 ++++++++++++++++++++-
5 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index abeb82b..6839e44 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -831,6 +831,22 @@ commit.template::
"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
specified user's home directory.
+commit.ignoreIntentToAdd::
+ When set to `false`, prevent `git commit` from creating a
+ commit from an index that has entries that were added with
+ `git add -N` but have not been updated with real contents, as
+ the user may have forgotten to tell the final contents for
+ these entries. Setting this to `true` makes `git commit`
+ pretend as if these entries do not exist in the index.
++
+The default for this variable is `false`, but it will change to `true`
+in future releases of git. To ease the transition, you may want to set
+it to `true` now and get used to the new behaviour early, or you may
+want to set it to `false` to keep the old behaviour a bit longer. We
+however expect to support setting this to `false` (to keep the current
+behaviour) only for a limited time after the default is changed to
+`true`.
+
credential.helper::
Specify an external helper to be called when a username or
password credential is needed; the helper may consult external
diff --git a/builtin/commit.c b/builtin/commit.c
index bf42bb3..da67653 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -86,6 +86,7 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
static int edit_flag = -1; /* unspecified */
static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
static int no_post_rewrite, allow_empty_message;
+static int cache_tree_flags;
static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
static char *sign_commit;
@@ -117,6 +118,8 @@ static enum {
} status_format = STATUS_FORMAT_LONG;
static int status_show_branch;
+static int set_commit_ignoreintenttoadd;
+
static int opt_parse_m(const struct option *opt, const char *arg, int unset)
{
struct strbuf *buf = opt->value;
@@ -400,7 +403,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(also ? prefix : NULL, pathspec, 0);
refresh_cache_or_die(refresh_flags);
- update_main_cache_tree(WRITE_TREE_SILENT);
+ update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
close_lock_file(&index_lock))
die(_("unable to write new_index file"));
@@ -420,8 +423,21 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
if (!pathspec || !*pathspec) {
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
+ if (!set_commit_ignoreintenttoadd) {
+ int i;
+ for (i = 0; i < active_nr; i++)
+ if (active_cache[i]->ce_flags & CE_INTENT_TO_ADD)
+ break;
+ if (i < active_nr) {
+ error(_("you intended to add \"%s\" but did not add it; not committing\n"
+ "hint: to commit all changes to tracked files, use \"commit -a\"\n"
+ "hint: to commit anyway without adding, set commit.ignoreIntentToAdd to true"),
+ active_cache[i]->name);
+ exit(128); /* die() */
+ }
+ }
if (active_cache_changed) {
- update_main_cache_tree(WRITE_TREE_SILENT);
+ update_main_cache_tree(cache_tree_flags | WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(&index_lock))
die(_("unable to write new_index file"));
@@ -870,7 +886,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
*/
discard_cache();
read_cache_from(index_file);
- if (update_main_cache_tree(0)) {
+ if (update_main_cache_tree(cache_tree_flags)) {
error(_("Error building trees"));
return 0;
}
@@ -1338,6 +1354,13 @@ static int git_commit_config(const char *k, const char *v, void *cb)
include_status = git_config_bool(k, v);
return 0;
}
+ if (!strcmp(k, "commit.ignoreintenttoadd")) {
+ set_commit_ignoreintenttoadd = 1;
+ if (git_config_bool(k, v))
+ cache_tree_flags |= WRITE_TREE_IGNORE_INTENT_TO_ADD;
+ else
+ cache_tree_flags &= ~WRITE_TREE_IGNORE_INTENT_TO_ADD;
+ }
status = git_gpg_config(k, v, NULL);
if (status)
diff --git a/cache-tree.c b/cache-tree.c
index 16355d6..d0be159 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -159,7 +159,9 @@ static int verify_cache(struct cache_entry **cache,
funny = 0;
for (i = 0; i < entries; i++) {
struct cache_entry *ce = cache[i];
- if (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {
+ if (ce_stage(ce) ||
+ ((flags & WRITE_TREE_IGNORE_INTENT_TO_ADD) == 0 &&
+ (ce->ce_flags & CE_INTENT_TO_ADD))) {
if (silent)
return -1;
if (10 < ++funny) {
@@ -339,8 +341,8 @@ static int update_one(struct cache_tree *it,
mode, sha1_to_hex(sha1), entlen+baselen, path);
}
- if (ce->ce_flags & CE_REMOVE)
- continue; /* entry being removed */
+ if (ce->ce_flags & (CE_REMOVE | CE_INTENT_TO_ADD))
+ continue; /* entry being removed or placeholder */
strbuf_grow(&buffer, entlen + 100);
strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
diff --git a/cache-tree.h b/cache-tree.h
index d8cb2e9..af3b917 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -38,6 +38,7 @@ int update_main_cache_tree(int);
#define WRITE_TREE_IGNORE_CACHE_TREE 2
#define WRITE_TREE_DRY_RUN 4
#define WRITE_TREE_SILENT 8
+#define WRITE_TREE_IGNORE_INTENT_TO_ADD 16
/* error return codes */
#define WRITE_TREE_UNREADABLE_INDEX (-1)
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index 58a3299..88a508e 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -41,7 +41,26 @@ test_expect_success 'cannot commit with i-t-a entry' '
echo frotz >nitfol &&
git add rezrov &&
git add -N nitfol &&
- test_must_fail git commit
+ test_must_fail git commit -minitial
+'
+
+test_expect_success 'can commit tree with i-t-a entry' '
+ git reset --hard &&
+ echo xyzzy >rezrov &&
+ echo frotz >nitfol &&
+ git add rezrov &&
+ git add -N nitfol &&
+ git config commit.ignoreIntentToAdd true &&
+ git commit -m initial &&
+ git ls-tree -r HEAD >actual &&
+ cat >expected <<EOF &&
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a elif
+100644 blob ce013625030ba8dba906f756967f9e9ca394464a file
+100644 blob cf7711b63209d0dbc2d030f7fe3513745a9880e4 rezrov
+EOF
+ test_cmp expected actual &&
+ git config commit.ignoreIntentToAdd false &&
+ git reset HEAD^
'
test_expect_success 'can commit with an unrelated i-t-a entry in index' '
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH v2 0/4] Deprecate "not allow as-is commit with i-t-a entries"
From: Nguyễn Thái Ngọc Duy @ 2012-02-07 12:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
- git-add.txt changes are removed. In the end all kinds of commit
behave the same way, not worth putting more explanation during the
transition.
- reword config text and warning text (or more precisely copy/paste
from Junio/Jonathan's words)
- Hard coded release numbers are removed. Now it's simply "in future".
- Step 2 may be too annoying. Users are warned on every commit if
commit.ignoreIntentToAdd is set. I think it's good because it keeps
config file clean, but people may think otherwise.
Nguyễn Thái Ngọc Duy (4):
cache-tree: update API to take abitrary flags
commit: introduce a config key to allow as-is commit with i-t-a entries
commit: turn commit.ignoreIntentToAdd to true by default
commit: remove commit.ignoreIntentToAdd, assume it's always true
builtin/commit.c | 9 ++++++---
cache-tree.c | 35 +++++++++++++++++------------------
cache-tree.h | 5 ++++-
merge-recursive.c | 2 +-
t/t2203-add-intent.sh | 21 ++++++++++++++++++++-
test-dump-cache-tree.c | 2 +-
6 files changed, 49 insertions(+), 25 deletions(-)
--
1.7.8.36.g69ee2
^ permalink raw reply
* [PATCH v2 1/4] cache-tree: update API to take abitrary flags
From: Nguyễn Thái Ngọc Duy @ 2012-02-07 12:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jonathan Nieder,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1328618804-31796-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/commit.c | 4 ++--
cache-tree.c | 27 ++++++++++++---------------
cache-tree.h | 4 +++-
merge-recursive.c | 2 +-
test-dump-cache-tree.c | 2 +-
5 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index eba1377..bf42bb3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -400,7 +400,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(also ? prefix : NULL, pathspec, 0);
refresh_cache_or_die(refresh_flags);
- update_main_cache_tree(1);
+ update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
close_lock_file(&index_lock))
die(_("unable to write new_index file"));
@@ -421,7 +421,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,
fd = hold_locked_index(&index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {
- update_main_cache_tree(1);
+ update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(&index_lock))
die(_("unable to write new_index file"));
diff --git a/cache-tree.c b/cache-tree.c
index 8de3959..16355d6 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -150,9 +150,10 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)
}
static int verify_cache(struct cache_entry **cache,
- int entries, int silent)
+ int entries, int flags)
{
int i, funny;
+ int silent = flags & WRITE_TREE_SILENT;
/* Verify that the tree is merged */
funny = 0;
@@ -241,10 +242,11 @@ static int update_one(struct cache_tree *it,
int entries,
const char *base,
int baselen,
- int missing_ok,
- int dryrun)
+ int flags)
{
struct strbuf buffer;
+ int missing_ok = flags & WRITE_TREE_MISSING_OK;
+ int dryrun = flags & WRITE_TREE_DRY_RUN;
int i;
if (0 <= it->entry_count && has_sha1_file(it->sha1))
@@ -288,8 +290,7 @@ static int update_one(struct cache_tree *it,
cache + i, entries - i,
path,
baselen + sublen + 1,
- missing_ok,
- dryrun);
+ flags);
if (subcnt < 0)
return subcnt;
i += subcnt - 1;
@@ -371,15 +372,13 @@ static int update_one(struct cache_tree *it,
int cache_tree_update(struct cache_tree *it,
struct cache_entry **cache,
int entries,
- int missing_ok,
- int dryrun,
- int silent)
+ int flags)
{
int i;
- i = verify_cache(cache, entries, silent);
+ i = verify_cache(cache, entries, flags);
if (i)
return i;
- i = update_one(it, cache, entries, "", 0, missing_ok, dryrun);
+ i = update_one(it, cache, entries, "", 0, flags);
if (i < 0)
return i;
return 0;
@@ -572,11 +571,9 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)
was_valid = cache_tree_fully_valid(active_cache_tree);
if (!was_valid) {
- int missing_ok = flags & WRITE_TREE_MISSING_OK;
-
if (cache_tree_update(active_cache_tree,
active_cache, active_nr,
- missing_ok, 0, 0) < 0)
+ flags) < 0)
return WRITE_TREE_UNMERGED_INDEX;
if (0 <= newfd) {
if (!write_cache(newfd, active_cache, active_nr) &&
@@ -672,10 +669,10 @@ int cache_tree_matches_traversal(struct cache_tree *root,
return 0;
}
-int update_main_cache_tree (int silent)
+int update_main_cache_tree (int flags)
{
if (!the_index.cache_tree)
the_index.cache_tree = cache_tree();
return cache_tree_update(the_index.cache_tree,
- the_index.cache, the_index.cache_nr, 0, 0, silent);
+ the_index.cache, the_index.cache_nr, flags);
}
diff --git a/cache-tree.h b/cache-tree.h
index 0ec0b2a..d8cb2e9 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -29,13 +29,15 @@ void cache_tree_write(struct strbuf *, struct cache_tree *root);
struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
int cache_tree_fully_valid(struct cache_tree *);
-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int, int);
+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int);
int update_main_cache_tree(int);
/* bitmasks to write_cache_as_tree flags */
#define WRITE_TREE_MISSING_OK 1
#define WRITE_TREE_IGNORE_CACHE_TREE 2
+#define WRITE_TREE_DRY_RUN 4
+#define WRITE_TREE_SILENT 8
/* error return codes */
#define WRITE_TREE_UNREADABLE_INDEX (-1)
diff --git a/merge-recursive.c b/merge-recursive.c
index d83cd6c..6479a60 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -264,7 +264,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)
if (!cache_tree_fully_valid(active_cache_tree) &&
cache_tree_update(active_cache_tree,
- active_cache, active_nr, 0, 0, 0) < 0)
+ active_cache, active_nr, 0) < 0)
die("error building trees");
result = lookup_tree(active_cache_tree->sha1);
diff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c
index e6c2923..a6ffdf3 100644
--- a/test-dump-cache-tree.c
+++ b/test-dump-cache-tree.c
@@ -59,6 +59,6 @@ int main(int ac, char **av)
struct cache_tree *another = cache_tree();
if (read_cache() < 0)
die("unable to read index file");
- cache_tree_update(another, active_cache, active_nr, 0, 1, 0);
+ cache_tree_update(another, active_cache, active_nr, WRITE_TREE_DRY_RUN);
return dump_cache_tree(active_cache_tree, another, "");
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: User authentication in GIT
From: Johan Herland @ 2012-02-07 12:32 UTC (permalink / raw)
To: supadhyay; +Cc: git
In-Reply-To: <1328615262741-7262113.post@n2.nabble.com>
On Tue, Feb 7, 2012 at 12:47, supadhyay <supadhyay@imany.com> wrote:
> Hi Robin and Jakub,
>
> Thanks for your reply. But I am still not getting what exactly I need to
> perform on GIT server. Please find my reply on your suggestion below:
>
>
> Robin:
> All users must have their own SSH key. You do not create keys for them.
> My rely: can you please give some more idea about how it works.. I am not
> getting this or if you can provide any link for this to understand.
- Each user generates their own ssh key pair on their own workstation
(in openssh, the command for generating a new key is called
'ssh-keygen')
- Each user then sends their public key to you (using email or
whatever communication form is easiest for you).
- You then load the keys into gitolite (by copying them into your
local clone of the gitolite-admin repo, committing, and pushing to the
gitolite-admin repo to the server).
More details here: http://sitaramc.github.com/gitolite/add.html (and
in associated documentation)
> Jakub:
> My reply: existing version control system used pserver protocol.
>
> You would still need for each user to generate their own SSH key.
> My reply: Do I need to store all end users sSH key in .ssh/authorized_keys
> file on GIT server?
No. You load them into gitolite (as described above, and in gitolite's
documentation), and then gitolite takes care of managing them.
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: User authentication in GIT
From: compufreak @ 2012-02-07 12:31 UTC (permalink / raw)
To: supadhyay; +Cc: git
In-Reply-To: <1328615262741-7262113.post@n2.nabble.com>
Inline respon
On Tue, Feb 7, 2012 at 12:47 PM, supadhyay <supadhyay@imany.com> wrote:
> Hi Robin and Jakub,
> ...
> Robin:
> All users must have their own SSH key. You do not create keys for them.
> My rely: can you please give some more idea about how it works.. I am not
> getting this or if you can provide any link for this to understand.
SSH authentication can use private/public keys. The user generates a
keypair on their computer and gives you their public key, the private
key stays on their computer.
>
> Jakub:
> My reply: existing version control system used pserver protocol.
>
> You would still need for each user to generate their own SSH key.
> My reply: Do I need to store all end users sSH key in .ssh/authorized_keys
> file on GIT server?
If you were to do it manually, yes. But if you use gitolite [1], then
you add them to another git repository which handles everything for
you.
> --
> View this message in context: http://git.661346.n2.nabble.com/User-authentication-in-GIT-tp7261349p7262113.html
> Sent from the git mailing list archive at Nabble.com.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[1]: https://github.com/sitaramc/gitolite
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox