Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries
From: Nguyen Thai Ngoc Duy @ 2012-02-07  0:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7vwr7z653f.fsf@alter.siamese.dyndns.org>

2012/2/7 Junio C Hamano <gitster@pobox.com>:
> It still is not clear to me how best to sell this change to the end-user
> community.
>
> IIRC, the original motivation of intent-to-add "add -N" was in response to
> users who curse Git because they often forget to add new files before
> committing, and they wanted to say "Here I have a new file, it is not
> complete yet, but I do not want it left out of the commit. While my memory
> is fresh, let me tell Git to keep an eye on it, so that it can remind me
> if I forget to add the final contents."  For them, the current "did you
> forget to add them?  If so tell me the final contents for at least the
> paths you will be changing with this commit" error was a perfect safety
> solution.
>
> It turned out that the benefits described we see above in the context,
> "This is useful, among other things, ...", were of more value, and for
> these use cases, i-t-a entries ceased to mean "I may forget, so I am
> telling you now, please remind me when I say commit."  And "did you
> forget?" error is hinderance for them.
>
> But does that mean nobody will ever need "please remind me lest I forget"?
> Just the original requestor of the "add -N" feature may still be using
> git, but more importantly, isn't it the case that those who have been
> using it merely for the other side effect (e.g. 'git diff') sometimes want
> the "please remind me" safety?
>
> I suspect that some among 1 million Git users would want the "please
> remind me", so a solution with configuration variable without command line
> override is not ideal (command line without any configurability is fine as
> long as we have a good default).

Which is why I prefer adding a new configuration variable (and
optionally a command line option) instead of deprecating current
behavior, because (being lazy) I never be able to find "some among 1
million" so I'm fine with assuming there are some among 1 million that
favors safety over convenience.

> I am beginning to think "safety by default, which can be turned off by
> learned users, but still can be turned on on demand" may be a lot easier
> to sell this.  That is:
>
>  - commit.ignoreIntentToAdd defaults to `false`; the default will never
>   change.  The users can set it to `true`.
>
>  - "commit --ignore-intent-to-add" can be used without setting the
>   configuration or to defeat an explicit `false`, for a one-shot request.
>
>  - "commit --honor-intent-to-add" can be used to defeat an explicit
>   `true`, for a one-shot request.
>
> The third one is a bit funny, as it is a way to bring back safety when the
> user earlier decided that he does not need that kind of safety (i.e. "I
> only say 'add -N' for `diff` and stuff, I will never forget to add real
> contents before committing"), so it will almost never be used, because
> these users who set 'ignoreIntentToAdd = true' do _not_ expect Git to help
> them in remembering to add the real contents.  And having to add a funny
> option just for the sake of completeness is often an indication that there
> is something fundamentally wrong in the system that the option tries to
> express an interface into it.

Well, that --honor-intent-to-add could be renamed as
--no-ignore-intent-to-add. The --[no-]ignore-intent-to-add pair
functions as a way to override default behavior/config var. No extra
could required. "git commit -h" just does not show it.

We need better option/config names though, --ignore-intent-to-add
looks way too long to type and it's not clear what it does without
looking up "git add -N".
-- 
Duy

^ permalink raw reply

* Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries
From: Jonathan Nieder @ 2012-02-07  0:59 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <CACsJy8Be=WLv1Zz5CqQMYfi8dJN6M980kVuckAQKAujVo8xFkQ@mail.gmail.com>

Nguyen Thai Ngoc Duy wrote:
> 2012/2/7 Junio C Hamano <gitster@pobox.com>:

>>> +                     if (i < active_nr)
>>> +                             warning(_("You are committing as-is with intent-to-add entries as the result of\n"
>>> +                                       "\"git add -N\". Git currently forbids this case.
>>
>> Can we phrase this a bit better?
>>
>> It is not like "forbids", but is "giving up because you didn't tell me
>> what content to include in the commit, even though you said you will tell
>> me later".
>
> "rejects"? I would rather say "see `git add -N` man page for more
> explanation" than putting it here. The warning is quite long as it is
> right now.

If I ruled the world, it would say something like this:

	error: you intended to add "foo.c" but did not add it; not committing
	hint: to commit all changes to tracked files, use "commit -a"
	hint: to commit without adding "foo.c", use "commit --ignore-intent-to-add", which may become the default in future versions of git

But without the long line. ;-)

^ permalink raw reply

* Re: [PATCH 3/4] commit: turn commit.ignoreIntentToAdd to true by default
From: Nguyen Thai Ngoc Duy @ 2012-02-07  1:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7v8vkf7mrn.fsf@alter.siamese.dyndns.org>

2012/2/7 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> -             if (!set_commit_ignoreintenttoadd) {
>
> Now no warning() is associated with testing of this flag, which means that
> people who didn't get around to read the doc and react to the warnings in
> the earlier releases will get _nothing_ when the real change hits them?
>
> That sounds strangely bad.  What am I missing?

Well if they stumble upon this case during the previous deprecation
phase, they ought to set commit.ignoreIntentToAdd. If it's not set,
they probably do not have prior experience with this feature. With
their fresh mind, they hopefully learn the new default behavior via
to-be-updated git-add.txt so no warnings for them.
-- 
Duy

^ permalink raw reply

* Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries
From: Junio C Hamano @ 2012-02-07  1:13 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git, Jonathan Nieder
In-Reply-To: <CACsJy8C5=JKHsjyaSFsxxyScb1CCQZmh4gXSAgsyJ8DFkjfQ0g@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> 2012/2/7 Junio C Hamano <gitster@pobox.com>:
>
>> I suspect that some among 1 million Git users would want the "please
>> remind me", so a solution with configuration variable without command line
>> override is not ideal (command line without any configurability is fine as
>> long as we have a good default).
>
> Which is why I prefer adding a new configuration variable (and
> optionally a command line option) instead of deprecating current
> behavior, because (being lazy) I never be able to find "some among 1
> million" so I'm fine with assuming there are some among 1 million that
> favors safety over convenience.

If there is one thing I want to absolutely avoid, it is to split the
userbase into many pieces by giving sticky configuration variables, so
the above argument is not a good starting point.

>> The third one is a bit funny, as it is a way to bring back safety when the
>> ....  And having to add a funny
>> option just for the sake of completeness is often an indication that there
>> is something fundamentally wrong in the system that the option tries to
>> express an interface into it.
>
> Well, that --honor-intent-to-add could be renamed as
> --no-ignore-intent-to-add.

I wasn't talking about the name at all.  What is _funny_ is the semantics.

A "by default unsafe" configuration introduces a need for an option to be
extra careful only when matters, but "an option" to be extra careful by
definition is easy to forget, so it is no longer a safety at all, iow,
people who want "by default unsafe" will get "always unsafe".  And it
probably is perfectly fine because to them, forgetting to add 'add -N'
entries is not a mistake at all, but always is a deliberate act.

Another thing I am somewhat worried about is if there are existing scripts
that create commits and relies on the current "we cannot commit because
the final contents is not known yet".  I didn't check but for example how
well does "git stash" work when the default is flipped to "just ignore"?

^ permalink raw reply

* Re: Git performance results on a large repository
From: Nguyen Thai Ngoc Duy @ 2012-02-07  1:19 UTC (permalink / raw)
  To: Sam Vilain
  Cc: Joshua Redstone, Ævar Arnfjörð Bjarmason,
	git@vger.kernel.org
In-Reply-To: <4F2C6276.1070100@vilain.net>

On Sat, Feb 4, 2012 at 5:40 AM, Sam Vilain <sam@vilain.net> wrote:
> There have also been designs at various times for sparse check–outs; ie
> check–outs where you don't check out the root of the repository but a
> sub–tree.

There is a sparse checkout feature in git (hopefully from one of the
designs you mentioned) and it can checkout subtrees. The only problem
in this case is it maintains full index. So it only solves half of the
problem (stat calls), reading/writing large index just slows
everything down.
-- 
Duy

^ permalink raw reply

* Re: Git performance results on a large repository
From: david @ 2012-02-07  1:28 UTC (permalink / raw)
  To: Joshua Redstone
  Cc: Nguyen Thai Ngoc Duy, Joey Hess, dgma@mohsinc.com, Matt Graham,
	Tomas Carnecky, Greg Troxel, David Barr, git@vger.kernel.org
In-Reply-To: <CB55A6A4.40AFD%joshua.redstone@fb.com>

On Mon, 6 Feb 2012, Joshua Redstone wrote:

> David Lang and David Barr, I generated the pack files by doing a repack:
> "git repack -a -d -f --max-pack-size=10g --depth=100 --window=250"  after
> generating the repo.

how many pack files does this end up creating?

I think that doing a full repack the way you did will group all revisions 
of a given file into a pack.

while what I'm saying is that if you create the packs based on time, 
rather than space efficiency of the resulting pack files, you may end up 
not having to go through as much date when doing things like a git blame.

what you did was

initialize repo
4M commits
repack

what I'm saying is

initialize repo
loop
    500K commits
    repack (and set pack to .keep so it doesn't get overwritten)

so you will end up with ~8 sets of pack files, but time based so that when 
you only need recent information you only look at the most recent pack 
file. If you need to go back through all time, the multiple pack files 
will be a little more expensive to process.

this has the added advantage that the 8 small repacks should be cheaper 
than the one large repack as it isn't trying to cover all commits each 
time.

David Lang

^ permalink raw reply

* Re: [PATCH 3/6] Stop producing index version 2
From: Shawn Pearce @ 2012-02-07  3:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nguyễn Thái Ngọc, git, Thomas Rast,
	Joshua Redstone
In-Reply-To: <7v4nv4a131.fsf@alter.siamese.dyndns.org>

2012/2/5 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> read-cache.c learned to produce version 2 or 3 depending on whether
>> extended cache entries exist in 06aaaa0 (Extend index to save more flags
>> - 2008-10-01), first released in 1.6.1. The purpose is to keep
>> compatibility with older git. It's been more than three years since
>> then and git has reached 1.7.9. Drop support for older git.
>
> Cc'ing this, as I suspect this would surely raise eyebrows of some people
> who wanted to get rid of the version 3 format.

Version 3 was a mistake because of the variable length record sizes.
Saving 2 bytes on some records that don't use the extended flags makes
the index file *MUCH* harder to parse. So much so that we should take
version 3 and kill it, not encourage it as the default!

IMHO, when these extended flags were added to make version 3 the
following should have happened:

- All records use the larger structure format with 4 bytes for the
flags, not 2 bytes.

- Change the trailing padding after the name to be a *SINGLE* \0 byte,
and do not pad out to an 8 byte boundary.

Both make it really hard to process the file, and the latter happens
only for direct mmap usage, which we don't do anymore.


We also have to consider the EGit and JGit user base as part of the
ecosystem. We can't just kill a file format because git-core has been
capable of reading its alternative since some arbitrary YYYY-MM-DD
release date. We need to also consider when did some other major tools
catch up and also support this format?

FWIW JGit released index version 3 support in version 0.9.1, which
shipped Sep 15, 2010. JGit/EGit were more than 2 years behind here.


<thinking type="wishful" probability="never-happen"
probably-inflating-flame-from="linus">

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. For one thing the index would be
able to register an empty subdirectory, rather than ignoring them. It
would also better line up with the filesystem's readdir() handling,
giving us more sane logic to compare what readdir() tells us exists
against what the index thinks should be in the same file. And the
overall index should be smaller, because we don't have to repeat the
same path/to/a/file/for/every/file/in/that/same/directory/tree.
Reconstructing the path strings at read time into a flat list should
be pretty trivial, and still keep the parallel lstat calls running off
a flat list working well for fast status operations.

</thinking>

^ permalink raw reply

* Re: [PATCH 5/6] Allow to use crc32 as a lighter checksum on index
From: Shawn Pearce @ 2012-02-07  3:17 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Thomas Rast, Joshua Redstone
In-Reply-To: <1328507319-24687-5-git-send-email-pclouds@gmail.com>

2012/2/5 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>        if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
>                return error("bad signature");
> -       if (hdr->hdr_version != htonl(2) &&
> -           hdr->hdr_version != htonl(3) &&
> -           hdr->hdr_version != htonl(4))
> +       if (hdr->hdr_version == htonl(2) ||
> +           hdr->hdr_version == htonl(3))
> +               do_crc = 0;
> +       else if (hdr->hdr_version == htonl(4)) {
> +               struct ext_cache_header *ehdr = (struct ext_cache_header *)hdr;
> +               do_crc = ntohl(ehdr->hdr_flags) & CACHE_F_CRC;
> +       }
> +       else

Ick. Ick. Ick. Please $DEITY no.

When it comes to data integrity codes in Git... PICK ONE AND STICK WITH IT.

If CRC-32 is good enough to protect the index content such that disk
corruption is probably detectable with it, lets just switch to CRC-32
in index version 4. Don't make it optional with a new header field
that wasn't there in version 3 and is now only able to accept 32 bits
of flags before we have to go and create index version 5. We already
have a cache extension system available with extension codes in the
footer of the index file. We don't need YET ANOTHER EXTENSION SYSTEM.

If CRC-32 is not good enough, and we don't want to trust it (or
really, YOU don't want to trust it) please do not then go and propose
that a less knowledgeable user should switch to CRC-32 "because it is
faster". If we don't want to rely on the error detection of CRC-32,
then we should be using SHA-1. Or SHA-256.


I haven't really put a lot of thought into this. But I suspect CRC-32
is sufficient on the index file, until it gets so big that the
probability of a bit flip going undetected is too high due to the size
of the file, but then we are into the "huge" index size range that has
you trying to swap out SHA-1 for CRC-32 because SHA-1 is too slow. Uhm
no.

CRC-32 may be good enough, we use it inside of the pack-objects when
doing repacking locally and don't want to inflate objects to check
SHA-1, but do want to try and detect a random bit flip caused by a
broken file copier. Thus far its held up well there. Given the very
transient nature of the index file (and how it can be mostly rebuilt
from a tree object and the working directory), CRC-32 might be good
enough. But please pick one.

^ permalink raw reply

* Re: [PATCH 5/6] Allow to use crc32 as a lighter checksum on index
From: Dave Zarzycki @ 2012-02-07  4:04 UTC (permalink / raw)
  To: Shawn Pearce
  Cc: Nguyễn Thái Ngọc Duy, git, Thomas Rast,
	Joshua Redstone
In-Reply-To: <CAJo=hJvSyhv8EUh=6ROotc3Q=zQo7vbww_ShQJP3tf1T7s889g@mail.gmail.com>

On Feb 6, 2012, at 7:17 PM, Shawn Pearce <spearce@spearce.org> wrote:

> I haven't really put a lot of thought into this. But I suspect CRC-32
> is sufficient on the index file, until it gets so big that the
> probability of a bit flip going undetected is too high due to the size
> of the file, but then we are into the "huge" index size range that has
> you trying to swap out SHA-1 for CRC-32 because SHA-1 is too slow. Uhm
> no.

CRCs are designed to be implemented in hardware and provide basic single-bit error checking for networking packets of disk blocks. With a good polynomial, they're reasonably effective at detecting a single-bit error within 8 or 16 kilobytes:

    http://www.ece.cmu.edu/~koopman/networks/dsn02/dsn02_koopman.pdf

^ permalink raw reply

* [PATCH] add -e: ignore dirty submodules
From: Johannes Schindelin @ 2012-02-07  4:05 UTC (permalink / raw)
  To: gitster; +Cc: git


We cannot add untracked/modified files in submodules anyway.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	This patch is actually from Oct 23, 2010.

 builtin/add.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 1c42900..b79336d 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -280,6 +280,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
 
 	argc = setup_revisions(argc, argv, &rev, NULL);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+	DIFF_OPT_SET(&rev.diffopt, IGNORE_DIRTY_SUBMODULES);
 	out = open(file, O_CREAT | O_WRONLY, 0644);
 	if (out < 0)
 		die (_("Could not open '%s' for writing."), file);
-- 
1.7.9.msysgit.0.27.ge92cd

^ permalink raw reply related

* Re: [PATCH 5/6] Allow to use crc32 as a lighter checksum on index
From: Dave Zarzycki @ 2012-02-07  4:29 UTC (permalink / raw)
  To: Shawn Pearce
  Cc: Nguyễn Thái Ngọc Duy, git, Thomas Rast,
	Joshua Redstone
In-Reply-To: <159A2D07-0B02-4E85-B7AA-C668FDA9F382@apple.com>


On Feb 6, 2012, at 8:04 PM, Dave Zarzycki <zarzycki@apple.com> wrote:

> On Feb 6, 2012, at 7:17 PM, Shawn Pearce <spearce@spearce.org> wrote:
> 
>> I haven't really put a lot of thought into this. But I suspect CRC-32
>> is sufficient on the index file, until it gets so big that the
>> probability of a bit flip going undetected is too high due to the size
>> of the file, but then we are into the "huge" index size range that has
>> you trying to swap out SHA-1 for CRC-32 because SHA-1 is too slow. Uhm
>> no.
> 
> CRCs are designed to be implemented in hardware and provide basic single-bit error checking for networking packets of disk blocks. With a good polynomial, they're reasonably effective at detecting a single-bit error within 8 or 16 kilobytes:
> 
>    http://www.ece.cmu.edu/~koopman/networks/dsn02/dsn02_koopman.pdf

s/packets of disk blocks/packets or disk blocks/g

^ permalink raw reply

* Re: [PATCH] git-add: allow --ignore-missing always, not just in dry run
From: Mike Gant @ 2012-02-07  4:39 UTC (permalink / raw)
  To: git; +Cc: Dieter Plaetinck
In-Reply-To: <1326923544-8287-1-git-send-email-dieter@plaetinck.be>

On Wed, Jan 18, 2012 at 10:52:24PM +0100, Dieter Plaetinck wrote:
> There is no need to restrict use of --ignore-missing to dry runs,
> it can be useful to ignore missing files during normal operation as
> well.

FWIW I would be in favor of this change and I was going to submit a
patch, too. My use case is different, though. I create branches that
will never be merged to the mainline because they have files added that
I don't want in master. The files added to these branches can vary. In
my script to create the branch, I want the largest possible set of these
files as the argument to 'git add' but if not all exist it's okay. I
realize I can write my script to only add the files that exist but I'm
lazy ;) and the --ignore-missing option would be easier.

Mike

^ permalink raw reply

* Re: [PATCH 3/6] Stop producing index version 2
From: Nguyen Thai Ngoc Duy @ 2012-02-07  4:50 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Junio C Hamano, git, Thomas Rast, Joshua Redstone
In-Reply-To: <CAJo=hJvtRnmvALcn3vKpYTr3j6ada8iboPjWN3cQnwwKzRvrDA@mail.gmail.com>

On Tue, Feb 7, 2012 at 10:09 AM, Shawn Pearce <spearce@spearce.org> wrote:
> 2012/2/5 Junio C Hamano <gitster@pobox.com>:
>> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>>
>>> read-cache.c learned to produce version 2 or 3 depending on whether
>>> extended cache entries exist in 06aaaa0 (Extend index to save more flags
>>> - 2008-10-01), first released in 1.6.1. The purpose is to keep
>>> compatibility with older git. It's been more than three years since
>>> then and git has reached 1.7.9. Drop support for older git.
>>
>> Cc'ing this, as I suspect this would surely raise eyebrows of some people
>> who wanted to get rid of the version 3 format.
>
> Version 3 was a mistake because of the variable length record sizes.
> Saving 2 bytes on some records that don't use the extended flags makes
> the index file *MUCH* harder to parse. So much so that we should take
> version 3 and kill it, not encourage it as the default!

Probably too late for that, but it's good to know there are strong
user base for v2.

> <thinking type="wishful" probability="never-happen"
> probably-inflating-flame-from="linus">
>
> 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. For one thing the index would be
> able to register an empty subdirectory, rather than ignoring them. It
> would also better line up with the filesystem's readdir() handling,
> giving us more sane logic to compare what readdir() tells us exists
> against what the index thinks should be in the same file. And the
> overall index should be smaller, because we don't have to repeat the
> same path/to/a/file/for/every/file/in/that/same/directory/tree.
> Reconstructing the path strings at read time into a flat list should
> be pretty trivial, and still keep the parallel lstat calls running off
> a flat list working well for fast status operations.
>
> </thinking>

Haven't really thought through, but I suppose we could create extended
tree object format (there is info in cache entry that's not in tree
entry), store index in this format, then pack together and store the
pack as part of index file. Append-only access to index would be
possible by appending a new pack of new trees to index) I think with
tree-based index, we could kill a big chunk of code (merging trees and
index together) in unpack_trees(). With further efforts to remove
list-based index usage, we could even kill match_pathspec_depth(),
making tree_entry_interesting() the only function to match patchspec.
But dreams probably never come true.
-- 
Duy

^ permalink raw reply

* Re: [PATCH 0/2] config includes, take 2
From: David Aguilar @ 2012-02-07  5:01 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20120206062713.GA9699@sigill.intra.peff.net>

On Sun, Feb 5, 2012 at 10:27 PM, Jeff King <peff@peff.net> wrote:
> That leaves the file-inclusion bits:
>
>  [1/2]: imap-send: remove dead code
>  [2/2]: config: add include directive
>
> The first patch is new in this round, and is a necessary cleanup for the
> second patch (though it might be worth applying regardless).
>
> The second patch corresponds to patch 1/4 from the previous round. Among
> the functional changes are:
>
>  1. do not use includes for "git config" in single-file mode

I have a questions about this.  Let's say I have ~/foo1.gitconfig:

[foo]
    bar = 1

...and ~/.gitconfig (I forgot the syntax):

[foo]
    bar = 0

#include "~/foo1.gitconfig"


Does that mean that:

    $ git config -f ~/.gitconfig foo.bar

...prints 0 and not 1?

I can see cases where this would be undesirable behavior.

For example, an application that uses "git config -z --list -f
~/.gitconfig" might expect that the result encompasses all of the
user-specific config bits.

Following this to its natural conclusion means "git config" might
learn some kind of --no-include flag for use with e.g. "git config
--no-include -f ~/.gitconfig".  That said, I don't know where I would
ever actually use such a flag.

I do know where I would use an `--include` flag (if following includes
were not the default behavior when using '-f'), though, and that's why
I'm asking.  The problem with not having it be the default means we
have to use a flag.  This makes it harder to support multiple versions
of git.

Maybe I'm mis-interpreting what you mean by, 'do not use includes for
"git config" in single-file mode', though.
--
David

^ permalink raw reply

* Re: [PATCH 0/2] config includes, take 2
From: Jeff King @ 2012-02-07  5:17 UTC (permalink / raw)
  To: David Aguilar; +Cc: git
In-Reply-To: <CAJDDKr6A2UvB3D-Dapw7WCEzWfzLoLd0E8MSDjT0RtdxFeWZAQ@mail.gmail.com>

On Mon, Feb 06, 2012 at 09:01:21PM -0800, David Aguilar wrote:

> I have a questions about this.  Let's say I have ~/foo1.gitconfig:
> 
> [foo]
>     bar = 1
> 
> ...and ~/.gitconfig (I forgot the syntax):
> 
> [foo]
>     bar = 0
> 
> #include "~/foo1.gitconfig"
> 
> 
> Does that mean that:
> 
>     $ git config -f ~/.gitconfig foo.bar
> 
> ...prints 0 and not 1?

Yes, though the syntax is:

  [include]
    path = foo1.gitconfig

(it doesn't respect tilde-expansion, but it probably should). Note that
the syntax was specifically selected for backwards compatibility, and to
allow manipulation with existing tools.

> I can see cases where this would be undesirable behavior.
> 
> For example, an application that uses "git config -z --list -f
> ~/.gitconfig" might expect that the result encompasses all of the
> user-specific config bits.

The problem is that an application might also expect it _not_ to happen
(e.g., if the application is really interested in knowing what's in
~/.gitconfig). Because includes aren't respected now, the safe default
seems to be not to have includes (i.e., don't change behavior for this
case).

A bigger question for me is: if you are interested in getting an answer
from anywhere, why are you restricting with "-f"? IOW, is this a
real-world problem, or a hypothetical one? And if real-world, what is
the actual use case?

> Following this to its natural conclusion means "git config" might
> learn some kind of --no-include flag for use with e.g. "git config
> --no-include -f ~/.gitconfig".  That said, I don't know where I would
> ever actually use such a flag.

It already learned it as part of my series (and --includes, as well).

> I do know where I would use an `--include` flag (if following includes
> were not the default behavior when using '-f'), though, and that's why
> I'm asking.  The problem with not having it be the default means we
> have to use a flag.  This makes it harder to support multiple versions
> of git.

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).

> Maybe I'm mis-interpreting what you mean by, 'do not use includes for
> "git config" in single-file mode', though.

No, I think you understand the described behavior.

-Peff

^ permalink raw reply

* Re: [PATCH 3/6] Stop producing index version 2
From: Junio C Hamano @ 2012-02-07  5:21 UTC (permalink / raw)
  To: Shawn Pearce
  Cc: Nguyễn Thái Ngọc, git, Thomas Rast,
	Joshua Redstone
In-Reply-To: <CAJo=hJvtRnmvALcn3vKpYTr3j6ada8iboPjWN3cQnwwKzRvrDA@mail.gmail.com>

Shawn Pearce <spearce@spearce.org> writes:

> <thinking type="wishful" probability="never-happen"
> probably-inflating-flame-from="linus">
>
> 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.

I suspect that is not so "never-happen wishful thinking".

In an earlier message, I alluded to a data structure that starts with a
single top-level tree entry that is lazily expanded as the index entries
are updated. The above shows that at least two of us share the same (day)
dream, and I suspect there are others that share the same "gut feeling"
that such a tree-based structure would be the way to do large index right.

It would be a large and possibly painful change, but the good thing is
that the index is a local matter and we won't have to worry too much about
a flag day event.

</thinking>

^ permalink raw reply

* Re: [PATCH] add -e: ignore dirty submodules
From: Junio C Hamano @ 2012-02-07  5:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.1202070504340.24259@s15462909.onlinehome-server.info>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> We cannot add untracked/modified files in submodules anyway.

I can see the updated code would not break "git apply" that will be run on
this output, but the above cannot be the whole story. It is unclear to me
what it is trying to achieve (in other words, "this patch does not break
the system" is not the whole purpose of the patch).

When a submodule is updated and is dirty, we would get:

    diff --git a/submodule b/submodule
    @@ -1,+1 @@
    -Subproject commit XXXX...
    +Subproject commit YYYY...-dirty

and leaving this diff in the edited patch adds YYYY... for submodule, even
though "-dirty" suffix is there.  So it is not fixing "the user tries to
update but fails because we do not filter dirty submodules" bug, or somesuch.
Besides, showing -dirty may be a good reminder that submodule has further
changes on top of what is going to be committed in this case.

When a submodule is only dirty, we would see:

    diff --git a/submodule b/submodule
    @@ -1,+1 @@
    -Subproject commit XXXX...
    +Subproject commit XXXX...-dirty

and leaving this diff in the edited patch keeps the submodule at XXXX...,
again without failing, so it is not fixing "the user gets unnecessary
error message" bug, or somesuch.  In this case, leaving this diff will be
a no-op so it is wasteful and distracting to the user who edits the patch.

Is that what this patch is about?  "For a submodule that is unchanged but
is dirty, submodule diff whose difference is only the '-dirty' suffix is
given but the user cannot update the index with such a diff anyway, so it
is a waste of space", or something like that?

That is the best guess I arrived at, but I suspect that it cannot be it,
as that discards the "-dirty" clue from the output when the submodule path
does have difference, as we saw in the earlier example. So there must be
something I am missing.

So I am out of ideas guessing what this patch is trying to achieve.  The
commit log shouldn't force the readers of the history to _guess_ like the
above.

>  builtin/add.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 1c42900..b79336d 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -280,6 +280,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
>  
>  	argc = setup_revisions(argc, argv, &rev, NULL);
>  	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
> +	DIFF_OPT_SET(&rev.diffopt, IGNORE_DIRTY_SUBMODULES);
>  	out = open(file, O_CREAT | O_WRONLY, 0644);
>  	if (out < 0)
>  		die (_("Could not open '%s' for writing."), file);

^ permalink raw reply

* Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries
From: Junio C Hamano @ 2012-02-07  6:10 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Jonathan Nieder
In-Reply-To: <7vwr7z653f.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> It still is not clear to me how best to sell this change to the end-user
> community.

OK, sorry for wavering.

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).

I was worried too much about backward compatibility and was blind to the
above, and was mistakenly defending a false "safety" that did not exist at
all.  Sorry for wasting everybody's time.

So let's bite the bullet and admit in the Release Notes that the current
behaviour was a UI mistake based on the misguided assumption that we can
give some kind of "safety" by committing when there are "add -N" entries
in the index, which is untrue, and we are fixing it in the new release.
We do not need configuration nor command line options.

Let me try to reroll your patch tomorrow (unless you beat me to it) and
see if I can come up with an easy-to-understand explanation to it.

Thanks.

^ permalink raw reply

* User authentication in GIT
From: supadhyay @ 2012-02-07  6:12 UTC (permalink / raw)
  To: git

Hi All,

I want to migrate my existing version control system (CVS) into GIT. The
first question which comes to me is in CVS we have user authentication like
username and their password while in GIT there is SSH authentication. 

Can any one suggest me what is the optimal way to manage the users in GIT?
Does all users having username,passowrd and SSH key? or thre is no users
credential but only SSH authentication? if I have 1000 users in old system
CVS then do I need to create a key for all 1000 users in GIT? or etc.


Thanks in advance...

--
View this message in context: http://git.661346.n2.nabble.com/User-authentication-in-GIT-tp7261349p7261349.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH 2/4] commit: introduce a config key to allow as-is commit with i-t-a entries
From: Nguyen Thai Ngoc Duy @ 2012-02-07  6:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jonathan Nieder
In-Reply-To: <7v1uq741ip.fsf@alter.siamese.dyndns.org>

2012/2/7 Junio C Hamano <gitster@pobox.com>:
> 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).

We need something in the commit message so that 5 years from now, when
someone raises the issue again, (s)he does not have to search the mail
archive. May I steal the above paragraph, maybe rephrase a little bit,
for commit message?
-- 
Duy

^ permalink raw reply

* [PATCHv2] tag: add --points-at list option
From: Tom Grennan @ 2012-02-07  7:01 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, jasampler
In-Reply-To: <20120206081119.GA3939@sigill.intra.peff.net>

The following is version 2 of the "points-at" feature.  I think that I've
addressed all of the comments on this discussion other than the name objection.
I suggest "of" instead, as in: "Show me the tags of...".
But, this may be too terse, so I welcome any other suggestions.

Note that this has been rebased onto pu for integration of 'jk/maint-tag-show-fixes'.

Thanks,

Tom Grennan (1):
  tag: add --points-at list option

 Documentation/git-tag.txt |    5 ++-
 builtin/tag.c             |   87 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 86 insertions(+), 6 deletions(-)

-- 
1.7.8

^ permalink raw reply

* [PATCHv2] tag: add --points-at list option
From: Tom Grennan @ 2012-02-07  7:01 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, jasampler
In-Reply-To: <20120206081119.GA3939@sigill.intra.peff.net>

This filters the list for annotated|signed tags of the given object.
Example,

   john$ git tag -s v1.0-john v1.0
   john$ git tag -l --points-at v1.0
   v1.0-john

Signed-off-by: Tom Grennan <tmgrennan@gmail.com>
---
 Documentation/git-tag.txt |    5 ++-
 builtin/tag.c             |   86 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5ead91e..97bedec 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git tag' [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>]
 	<tagname> [<commit> | <object>]
 'git tag' -d <tagname>...
-'git tag' [-n[<num>]] -l [--contains <commit>]
+'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
 	[--column[=<options>] | --no-column] [<pattern>...]
 'git tag' -v <tagname>...
 
@@ -95,6 +95,9 @@ This option is only applicable when listing tags without annotation lines.
 --contains <commit>::
 	Only list tags which contain the specified commit.
 
+--points-at <object>::
+	Only list annotated or signed tags of the given object.
+
 -m <msg>::
 --message=<msg>::
 	Use the given tag message (instead of prompting).
diff --git a/builtin/tag.c b/builtin/tag.c
index 5fbd62c..a1d3a04 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -20,17 +20,34 @@
 static const char * const git_tag_usage[] = {
 	"git tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
 	"git tag -d <tagname>...",
-	"git tag -l [-n[<num>]] [<pattern>...]",
+	"git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>] \\"
+		"\n\t\t[<pattern>...]",
 	"git tag -v <tagname>...",
 	NULL
 };
 
+struct points_at {
+	struct points_at *next;
+	unsigned char *sha1;
+};
+
 struct tag_filter {
 	const char **patterns;
 	int lines;
 	struct commit_list *with_commit;
+	struct points_at *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;
+	}
+}
+
 static unsigned int colopts;
 
 static int match_pattern(const char **patterns, const char *ref)
@@ -44,6 +61,29 @@ static int match_pattern(const char **patterns, const char *ref)
 	return 0;
 }
 
+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;
+}
+
 static int in_commit_list(const struct commit_list *want, struct commit *c)
 {
 	for (; want; want = want->next)
@@ -141,6 +181,10 @@ static int show_reference(const char *refname, const unsigned char *sha1,
 				return 0;
 		}
 
+		if (filter->points_at
+		    && !match_points_at(filter->points_at, sha1))
+			return 0;
+
 		if (!filter->lines) {
 			printf("%s\n", refname);
 			return 0;
@@ -154,16 +198,19 @@ static int show_reference(const char *refname, const unsigned char *sha1,
 }
 
 static int list_tags(const char **patterns, int lines,
-			struct commit_list *with_commit)
+			struct commit_list *with_commit,
+			struct points_at *points_at)
 {
 	struct tag_filter filter;
 
 	filter.patterns = patterns;
 	filter.lines = lines;
 	filter.with_commit = with_commit;
+	filter.points_at = points_at;
 
 	for_each_tag_ref(show_reference, (void *) &filter);
 
+	free_points_at(points_at);
 	return 0;
 }
 
@@ -389,12 +436,33 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
 	return check_refname_format(sb->buf, 0);
 }
 
+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;
+}
+
 int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf ref = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
+	struct points_at *points_at = NULL;
 	struct ref_lock *lock;
 	struct create_tag_options opt;
 	char *cleanup_arg = NULL;
@@ -432,6 +500,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			PARSE_OPT_LASTARG_DEFAULT,
 			parse_opt_with_commit, (intptr_t)"HEAD",
 		},
+		{
+			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,
+		},
 		OPT_END()
 	};
 
@@ -471,15 +545,17 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 			copts.padding = 2;
 			run_column_filter(colopts, &copts);
 		}
-		ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit);
+		ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit,
+				points_at);
 		if (lines == -1 && colopts & COL_ENABLED)
 			stop_column_filter();
 		return ret;
 	}
 	if (lines != -1)
 		die(_("-n option is only allowed with -l."));
-	if (with_commit)
-		die(_("--contains option is only allowed with -l."));
+	if (with_commit || points_at)
+		die(_("--contains and --points-at options "
+		      "are only allowed with -l."));
 	if (delete)
 		return for_each_tag_name(argv, delete_tag);
 	if (verify)
-- 
1.7.8

^ permalink raw reply related

* cloning a tree which has detached branch checked out
From: Michael S. Tsirkin @ 2012-02-07  7:06 UTC (permalink / raw)
  To: git

I have a tree where I checked out a remote, without
creating a local branch, this detaches the HEAD:
# git branch
* (no branch)
  master
#git log|head -1
commit cec64082f689f949a397cb9b39423dc41545fa0e
#git log master..HEAD|head -1
commit cec64082f689f949a397cb9b39423dc41545fa0e


Now if I try to clone it:
#git clone -n lab:/home/mst/scm/linux
Initialized empty Git repository in /home/mst/scm/linux/.git/
remote: Counting objects: 1693446, done.
remote: Compressing objects: 100% (277054/277054), done.
Receiving objects: 100% (1693446/1693446), 418.16 MiB | 7.63 MiB/s,
done.
remote: Total 1693446 (delta 1414353), reused 1681280 (delta 1402358)
Resolving deltas: 100% (1414353/1414353), done.
error: Trying to write ref HEAD with nonexistant object
cec64082f689f949a397cb9b39423dc41545fa0e
fatal: Cannot update the ref 'HEAD'.

Looks like a bug, doesn't it?

-- 
MST

^ permalink raw reply

* Re: User authentication in GIT
From: Robin H. Johnson @ 2012-02-07  7:30 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <1328595129258-7261349.post@n2.nabble.com>

On Mon, Feb 06, 2012 at 10:12:09PM -0800,  supadhyay wrote:
> Hi All,
[snip]

1. Go and use gitolite.
2. All users must have their own SSH key. You do not create keys for them.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85

^ permalink raw reply

* Re: [PATCH] add -e: ignore dirty submodules
From: Johannes Schindelin @ 2012-02-07  7:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaa4v42fi.fsf@alter.siamese.dyndns.org>

Dear Junio C Hamano,

On Mon, 6 Feb 2012, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > We cannot add untracked/modified files in submodules anyway.

The purpose of "add -e" is to stage changes. It does so by presenting a
diff which the user can edit (and applying it after recounting the numbers
in the hunk headers).

If the diff does not apply, it does not make sense to present it to the
user.

Offering to add changes represented by a diff like

diff --git <blub>
<header>
-deadbeef...
+deadbeef...-dirty

does not make sense.

Even a diff like

diff --git <blah>
<header>
-deadbeef...
-coffeebabe...-dirty

would be refused by git-apply.

Indeed, due to the design of the submodules, it is impossible to stage
dirty files in a submodule and a supercommit at the same time. Oh, and this
discussion is not the place to wish for a feature like that, just in case
you want to ask me to implement that in order to be allowed to have my puny
little patch applied. (I guess this is the reason why I waited so long
before I dared to submit the patch to the mailing list.)

Now, due to these concerns, even stripping out the -dirty part can lead to
a comically non-sensical diff like

diff --git <blergh>
<header>
-deadbeef...
+deadbeef...

So keeping the diff generation as-is but preparing the patch by munching
away the -dirty suffix would not fix the problem.

Also, it would be wrong to assume that the user asked to get a status
update. git add -e != git status. If the user wanted to know what changes
are in the worktree, including the worktrees of the submodules, but only
those that have been checked out, git status would be the command to call
(even if it was touted as a git commit --dry-run once upon a time, which
is kind of wrong, see above, you cannot commit the untracked or dirty
files in a submodule, yet git status shows them).

So: showing the fact that a submodule has untracked or dirty files in the
patch that the user wants to edit with git add -e is wrong, wrong, wrong.

The only submodule-related thing we should ever present to the user who
called git add -e is a diff like

diff --git <narf>
<header>
-deadcad...
+beda551ed...

because that is a stageable change.

Alas, salvation is nigh! Yes, just a little line which asks for the level
"dirty" (which implies "untracked", as detailed by diff-options.txt, uhm,
sorry, I was asked to be precise, Documentation/diff-options.txt) fixes
that!

With this flag, there are no more "-dirty" lines in the submodule diffs.
None! Only diffs between different submodule commits are shown, and even
if the worktree of the submodule is dirty, or has untracked files, the
-dirty suffix is omitted. Just what you need for git-apply to work! Yay!

As a plus, it makes the diff generation faster because what cannot be
staged anyway is not even discovered! Super-yay!

I actually enjoyed writing this text so much, may I respectfully ask to
include it verbatim in the commit message?

Thank you very much,
Johannes Schindelin

^ permalink raw reply


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