Git development
 help / color / mirror / Atom feed
* Re: RFD: leveraging GitHub's asciidoc rendering for our Documentation/
From: Scott Chacon @ 2011-09-09 15:45 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <4E6A23DB.1040606@drmicha.warpmail.net>

Hey,

On Fri, Sep 9, 2011 at 7:34 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> which has all the renaming (*.txt -> *.asciidoc) and Makefile and script
> changes, but is missing some include changes (because include breaks
> anyway, see below).

I can change this so we can render .asc if that's less ugly.  I've
been meaning to do this for a while, but I don't think I ever
incorporated it.

>
> Our own customisation is not loaded (of course) so that, e.g., the
> linkgit macro does not work; and the include statement makes GitHub's
> parser unhappy and choke.
>
> Does anybody feel this is worth pursuing?
>
> + Nicer blob view
> + Simpler way to judge documentation changes
> - Need to get our asciidoc config in there
> - GitHub's parser neeeds to learn include

If this is interesting to people, I can help out.  However, you can
also fix rendering issues, as long as they don't introduce security
issues (which 'include' and macros can do) by fixing it here:

https://github.com/github/markup

That is the rendering engine we use.  If you make it work how you want
and it's safe and you send us a pull request, we'll happily take fixes
to the asciidoc renderer.

Scott

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Michael Haggerty @ 2011-09-09 15:51 UTC (permalink / raw)
  To: Martin Fick; +Cc: git
In-Reply-To: <4E6A19AD.80100@alum.mit.edu>

I have answered some of my own questions:

On 09/09/2011 03:50 PM, Michael Haggerty wrote:
> 3. Try using the --no-replace-objects option (I assume that it can be
> used like "git --no-replace-objects fetch ...").  In my case this option
> made a dramatic improvement in the runtimes.

This does not seem to help much.

> 4. Try a test using a repository generated something like the test
> script that I posted in [1].  If it also gives pathologically bad
> performance, then it can serve as a test case to use while we debug the
> problem.

Yes, a simple test repo like that created by the script is enough to
reproduce the problem.  The slowdown becomes very obvious after only a
few hundred references.

Curiously, "git clone" is very fast under the same circumstances that
"git fetch" is excruciatingly slow.

According to strace, git seems to be repopulating the ref cache after
each new ref is created (it walks through the whole refs subdirectory
and reads every file).  Apparently the ref cache is being discarded
completely whenever a ref is added (which can and should be fixed) and
then being reloaded for some reason (though single refs can be inspected
much faster without reading the cache).  This situation should be
improved by the hierarchical refcache changes that I'm working on plus
smarter updating (rather than discarding) of the cache when a new
reference is created.

Some earlier speculation in this thread was that that slowdowns might be
caused by "pessimal" ordering of revisions in the walker queue.  But my
test repository shards the references in such a way that the lexical
order of the refnames does not correspond to the topological order of
the commits.  So that can't be the whole story.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Fwd: RFD: leveraging GitHub's asciidoc rendering for our Documentation/
From: Sitaram Chamarty @ 2011-09-09 15:55 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <CAMK1S_hOY-riZnHAZhC32UAA0BYF1YyXUPujj_jFUEjcYC_4ZA@mail.gmail.com>

forgot to copy the list; sorry...


---------- Forwarded message ----------
From: Sitaram Chamarty <sitaramc@gmail.com>
Date: Fri, Sep 9, 2011 at 9:24 PM
Subject: Re: RFD: leveraging GitHub's asciidoc rendering for our Documentation/
To: Michael J Gruber <git@drmicha.warpmail.net>


On Fri, Sep 9, 2011 at 8:04 PM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Hi there,
>
> I've been looking more to GitHub lately and was wondering whether it is
> worth to leverage their automatic asciidoc rendering for our asciidoc
> files. I have put up a test tree at
>
> https://github.com/gitigit/git/tree/githubtest
>
> which has all the renaming (*.txt -> *.asciidoc) and Makefile and script
> changes, but is missing some include changes (because include breaks
> anyway, see below).
>
> The simple renaming already gives a rendered display of blobs for
> simpler asciidoc files like release notes
>
> https://github.com/gitigit/git/blob/githubtest/Documentation/RelNotes/1.7.7.asciidoc
>
> and api documentation
>
> https://github.com/gitigit/git/blob/githubtest/Documentation/technical/api-credentials.asciidoc
>
> For the man pages, there are several problems as can be seen here:
>
> https://github.com/gitigit/git/blob/githubtest/Documentation/git-blame.asciidoc
>
> Our own customisation is not loaded (of course) so that, e.g., the
> linkgit macro does not work; and the include statement makes GitHub's
> parser unhappy and choke.

maybe github will consider supporting linkgit?  3 letters are common anyway :)

> Does anybody feel this is worth pursuing?

For a long time, I relied on github's rendering for all of my
(gitolite) documentation.  Eventually I realised it is too slow to
render.  More importantly, the whole github "presence" was extraneous
to the manpage, and often distracted my readers.

Eventually I started pre-rendering my documentation to HTML myself and
pushing it to a branch called "gh-pages".  Contrast the visual appeal
of the github-rendered page [1] versus the pre-rendered page [2].  (I
admit I do have some very minimal CSS in my version but that's another
plus point for pre-rendering)

[1]: https://github.com/sitaramc/gitolite/blob/pu/README.mkd
[2]: http://sitaramc.github.com/gitolite/README.html

-- 
Sitaram

^ permalink raw reply

* Re: RFD: leveraging GitHub's asciidoc rendering for our Documentation/
From: Michael Haggerty @ 2011-09-09 15:58 UTC (permalink / raw)
  To: Scott Chacon; +Cc: Michael J Gruber, Git Mailing List
In-Reply-To: <CAP2yMaJ+UBHZp0U=QWOZbnbbOuq8vNoJODWtxZPb1sKXTC_UEg@mail.gmail.com>

On 09/09/2011 05:45 PM, Scott Chacon wrote:
> On Fri, Sep 9, 2011 at 7:34 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> which has all the renaming (*.txt -> *.asciidoc) and Makefile and script
>> changes, but is missing some include changes (because include breaks
>> anyway, see below).
> 
> I can change this so we can render .asc if that's less ugly.  I've
> been meaning to do this for a while, but I don't think I ever
> incorporated it.

What about letting the project set a gitattribute that tells github how
to render particular files?  It would not require files to be renamed,
and it would be more flexible.

OTOH it would not be possible to apply gitattributes (or file renamings)
to old revisions, so the history would continue to be rendered naively.
 But here's an additional idea: github could provide web access to the
equivalent of $GIT_DIR/info/attributes (a project-wide .gitattributes
file), which would allow the rendering of files in historical revisions
to be customized and would also allow github rendering behavior to be
defined even in projects that do not want github-specific tags in the
.gitattributes files in their project.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Jens Lehmann @ 2011-09-09 15:59 UTC (permalink / raw)
  To: Martin Fick; +Cc: git
In-Reply-To: <1315529522448-6774328.post@n2.nabble.com>

Am 09.09.2011 02:52, schrieb Martin Fick:
> An update, I bisected it down to this commit:
> 
>   88a21979c5717e3f37b9691e90b6dbf2b94c751a
> 
>    fetch/pull: recurse into submodules when necessary
> 
> Since this can be disabled with the --no-recurse-submodules switch, I tried
> that and indeed, even with the latest 1.7.7rc it becomes fast (~8mins)
> again. The strange part about this is that the repository does not have any
> submodules. Anyway, I hope that this can be useful to others since it is a
> workaround which speed things up enormously. Let me know if you have any
> other tests that you want me to perform,

Thanks for nailing that one down. I'm currently looking into bringing back
decent performance here.

^ permalink raw reply

* Re: Git Bug - diff in commit message.
From: Junio C Hamano @ 2011-09-09 16:00 UTC (permalink / raw)
  To: Michael Witten; +Cc: anikey, git
In-Reply-To: <CAMOZ1BtbpbG+19G6Hfau_2_F5L3Ad+x-Payd9aKajJxU_V_tyA@mail.gmail.com>

Michael Witten <mfwitten@gmail.com> writes:

> It would appear that `git rebase' is in fact producing patches with
> `git format-patch' and then applying the resulting patches with `git
> am', which gets confused by your inline diff; this can be clearly seen
> in the `git-rebase--am[.sh]' file.
>
> Perhaps `git rebase' should be reimplemented to use `git cherry-pick',
> or does that suffer from the same problem?

I think it just is a simple matter of this one-liner.  We were already
bending backwards to preserve the original message by not parsing and
running stripspace the message in the output from mailinfo, and instead
using the log message from the original, but were still using the patch
text that came from mailinfo that was split incorrectly because it was
fooled by the diff in the commit log message.

In the longer term, I think "git-rebase--am.sh" should be rewritten to
have format-patch avoid the cost of actually generating the patch text,
and the "mailinfo" call that comes above the context shown in this patch
should be made conditional---when using "am" for rebasing we do not really
care anything but the commit object names, and everything else is figured
out from the commit this codepath.

 git-am.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index 016b505..9a4cb2d 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -632,6 +632,7 @@ To restore the original branch and stop patching run \"\$cmdline --abort\"."
 			sed -e '1,/^$/d' >"$dotest/msg-clean"
 			echo "$commit" > "$dotest/original-commit"
 			get_author_ident_from_commit "$commit" > "$dotest/author-script"
+			git diff-tree --root --binary -m --first-parent "$commit" >"$dotest/patch"
 		else
 			{
 				sed -n '/^Subject/ s/Subject: //p' "$dotest/info"

^ permalink raw reply related

* Re: [PATCHv2 4/5] branch: introduce --list option
From: Junio C Hamano @ 2011-09-09 16:02 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Jeff King, git
In-Reply-To: <4E69B832.9030503@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Junio C Hamano venit, vidit, dixit 08.09.2011 23:17:
>> Ah, nevermind.
>> 
>> As the series is already in 'next', here is what I came up with.
>
> I thought you'll rebuild next anyways after 1.7.7, but either way it's
> fine. Thanks for holding this series in next long enough to really cook
> it (and Jeff for revisiting it), it's much better now, keeping the
> (undocumented, but expected) behavior of "git branch -v foo".

Thank *you* for all the work.

I recall Peff had some comments on your new tests last night, by the way.

^ permalink raw reply

* Re: [PATCH 2/2] push -s: skeleton
From: Joey Hess @ 2011-09-09 16:03 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <robbat2-20110909T004300-810527870Z@orbis-terrarum.net>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="//TRANSLIT", Size: 2109 bytes --]

Robin H. Johnson wrote:
> Joey Hess discussed this two years ago, and again last week:
> http://kitenet.net/~joey/blog/entry/size_of_the_git_sha1_collision_attack_surface/
> 
> This is easy in the kernel tree, it's got lots of eyeballs and only few
> binary files. This isn't true for lots of other Git trees, a tree with a
> JPEG image or a gzip file would be a great target.

The most credible attack I have so far does not involve binary files in
tree. Someone pointed out that git log, git show, etc stop printing
commit messages at NULL. So colliding binary garbage can be put in a
commit message and be unlikely to be noticed, and the commit can
later be altered to point to a different tree.

https://github.com/joeyh/supercollider

joey@gnu:~/tmp/supercollider>git log
commit 24f30db5790b209fa412ce81c5ef2bf8af5fd4d7
Author: Joey Hess <joey@kitenet.net>
Date:   Fri Sep 9 11:49:21 2011 -0400

    an innocent commit
    
    If this were a sha1 colliding attack, there would be some sort of binary
    garbage below. Which there isn't. So this can be safely merged.
joey@gnu:~/tmp/supercollider>git cat-file commit 24f30db5790b209fa412ce81c5ef2bf8af5fd4d7
tree 735a7633237c07b398856005de3bc9ea00446747
author Joey Hess <joey@kitenet.net> 1315583361 -0400
committer Joey Hess <joey@kitenet.net> 1315583361 -0400

an innocent commit

If this were a sha1 colliding attack, there would be some sort of binary
garbage below. Which there isn't. So this can be safely merged.
\0


??b???\x1f[?i??ͯ?t?\f2??\x02????os?\x14<????h?+,M?mY?e?EW?i\x13v$???\x14J??U}n~???L??????f??\x02?ě??3>?Q??H?޸\x16*zl\x1a?RA˂q?E\f?\x06\x16E\x7f7??^[?\x03\?m???U?\x1e>MU\v	GY?d)?ȼ??'g?~D??ɯhQ?\x13???/"E\x04??X?m???^͸??S?D\x13??;w6(?`??>?\x10縘?\aAѲ?*!??@v????>?8??2\b?\x14!??=*?J	^[\r\r???\x01ynH\x10???c?w?\??K7??\x1c?N?6??\x1c???A5?FM?wZ?~?pK\x02Y?R???s7\x7f??(?\aƶ?_"??m\x11%????\x7f1\x7fa??ʀ??K[\rt??\x11??\x0e!A0?ΈfT.?T?w\a?򁛵ƌ\v?р???aco?V/2\x14??nَ?
?}?6?\x19_?z?{

It might be worth ameloriating that attack by making git log always
show the full buffer. Or it would be easy to write a tool that finds
any commits that have a NULL in their message.

-- 
see shy jo

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

^ permalink raw reply

* Re: Git is not scalable with too many refs/*
From: Jens Lehmann @ 2011-09-09 16:03 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Martin Fick, git
In-Reply-To: <4E6A19AD.80100@alum.mit.edu>

Am 09.09.2011 15:50, schrieb Michael Haggerty:
> On 09/08/2011 09:53 PM, Martin Fick wrote:
>> Just thought that I should add some numbers to this thread as it seems that
>> the later versions of git are worse off by several orders of magnitude on
>> this one.  
>>
>> We have a Gerrit repo with just under 100K refs in refs/changes/*.  When I
>> fetch them all with git 1.7.6 it does not seem to complete.  Even after 5
>> days, it is just under half way through the ref #s! [...]
> 
> I recently reported very slow performance when doing a "git
> filter-branch" involving only about 1000 tags, with hints of O(N^3)
> scaling [1].  That could certainly explain enormous runtimes for 100k refs.
> 
> References are cached in git in a single linked list, so it is easy to
> imagine O(N^2) all over the place (which is bad enough for 100k
> references).  I am working on improving the situation by reorganizing
> how the reference cache is stored in memory, but progress is slow.
> 
> I'm not sure whether your problem is related.  For example, it is not
> obvious to me why the commit that you cite (88a21979) would make the
> reference problem so dramatically worse.

88a21979 is the reason, as since then a "git rev-list <sha1> --not --all" is
run for *every* updated ref to find out all new commits fetched for that ref.
And if you have 100K of them ...

^ permalink raw reply

* Re: git repository size / compression
From: John Szakmeister @ 2011-09-09 16:05 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: neubyr, git
In-Reply-To: <1315556595.2019.11.camel@bee.lab.cmartin.tk>

On Fri, Sep 9, 2011 at 4:23 AM, Carlos Martín Nieto <cmn@elego.de> wrote:
[snip]
>> Subversion repo/server:
>> {{{
>> $ du -h -d 1
>>  12K    ./conf
>> 1.2M    ./db
>>  36K    ./hooks
>> 8.0K    ./locks
>> 1.2M    .
>> }}}
>
> I don't know how the repository is stored in Subversion, but it may also
> be compressed. You may be able to reduced your git repository size by
> (re)generating packs with 'git repack' and doing some cleanups with 'git
> gc', but the repository size is not often a concern.

It is stored compressed in Subversion, and it also generates deltas
against previous versions.  IIRC, the delta algorithm in an xdelta
based one, and then the data is run through compression.  Subversion
will at times choose to self-compress the file, instead of doing a
delta and compressing.  IIRC, there is some heuristics in there for
determining when to do that, but I forget the exact method.

HTH!

-John

^ permalink raw reply

* Re: RFD: leveraging GitHub's asciidoc rendering for our Documentation/
From: Michael J Gruber @ 2011-09-09 16:05 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Scott Chacon, Git Mailing List
In-Reply-To: <4E6A378E.6020704@alum.mit.edu>

Michael Haggerty venit, vidit, dixit 09.09.2011 17:58:
> On 09/09/2011 05:45 PM, Scott Chacon wrote:
>> On Fri, Sep 9, 2011 at 7:34 AM, Michael J Gruber
>> <git@drmicha.warpmail.net> wrote:
>>> which has all the renaming (*.txt -> *.asciidoc) and Makefile and script
>>> changes, but is missing some include changes (because include breaks
>>> anyway, see below).
>>
>> I can change this so we can render .asc if that's less ugly.  I've
>> been meaning to do this for a while, but I don't think I ever
>> incorporated it.
> 
> What about letting the project set a gitattribute that tells github how
> to render particular files?  It would not require files to be renamed,
> and it would be more flexible.
> 
> OTOH it would not be possible to apply gitattributes (or file renamings)
> to old revisions, so the history would continue to be rendered naively.
>  But here's an additional idea: github could provide web access to the
> equivalent of $GIT_DIR/info/attributes (a project-wide .gitattributes
> file), which would allow the rendering of files in historical revisions
> to be customized and would also allow github rendering behavior to be
> defined even in projects that do not want github-specific tags in the
> .gitattributes files in their project.
> 
> Michael

I don't think that the naming is a problem. In fact, we have .txt files
which are asciidoc and some which are not, so renaming the former is an
improvement in itself.

Also, I don't mean to replace our prerendered html. Just a nicer source
view for documentation source.

Since this is about source view, we might not even want to execute an
include - but we don't want the rendering to stop where there is one either.

Besides linkgit and things like apostrophe, all our asciidoc marcos are
workarounds for docbook, and thus a non-issue for html rendering.

Don't know where the parser gets stuck. Maybe cwd is not where one
thinks it is, and safe mode spoils the party.

Michael

^ permalink raw reply

* Re: [PATCH 2/2] push -s: skeleton
From: Drew Northup @ 2011-09-09 16:14 UTC (permalink / raw)
  To: Joey Hess; +Cc: Git Mailing List, Robin H. Johnson, Shawn O. Pearce, Jeff King
In-Reply-To: <20110909160301.GA9707@gnu.kitenet.net>


On Fri, 2011-09-09 at 12:03 -0400, Joey Hess wrote:

> It might be worth ameloriating that attack by making git log always
> show the full buffer. Or it would be easy to write a tool that finds
> any commits that have a NULL in their message.

Just be aware that fixing the problem by diallowing NULLs in the commit
message makes it impossible to include UTF-16 text in the commit message
(which isn't currently handled very well anyway). Granted, I'm not sure
what the most decent way of dealing with that otherwise might be as I've
not taken the time to think about it...

-- 
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

^ permalink raw reply

* Re: [PATCH] Disable useless check_for_new_submodule_commits() by default
From: Jens Lehmann @ 2011-09-09 16:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Heiko Voigt
In-Reply-To: <7vy5xyv5p3.fsf@alter.siamese.dyndns.org>

Am 09.09.2011 03:56, schrieb Junio C Hamano:
> Imagine a project with 20k paths with a history 250k commits deep, without
> any submodule.
> 
> Imagine another project with the same depth of history but with 1k paths,
> among which there are 200 submodules.
> 
> Further, imagine fetching from one of the above repositories into a
> repository that is very behind, and updating many remote tracking
> branches.  Now, think about what check_for_new_submodule_commits() in
> submodule.c that is called from update_local_ref() in builtin/fetch.c
> would do.
> 
> For each updated remote tracking branch (or anything that is not a tag),
> the problem function will run the equivalent of:
> 
> 	git log --raw $new --not --all
> 
> which would mean 250k rounds of diff-tree to enumerate the submodules that
> may have been updated, and it does it for each and every refs outside the
> refs/tags hierarchy.
>
> Presumably, this is so that it only has to actually fetch in the submodule
> "on demand", but even if in a project _with_ submodules (i.e. the latter
> example above), this is simply not acceptable.

Definitely.

> You could just enumerate
> those 200 submodules at the tip that _might_ matter and that would be
> million times cheaper.

But that trick won't do it. I thought about that too when I implemented this
feature, but we need to look in all new commits (e.g. a submodule is not
present anymore in the new tip but is changed in the preceding but not yet
fetched commits, then it still has to be checked if it has the necessary
commits locally and they need to be fetched if that is not the case).

> To add insult to injury, this "on demand" behaviour
> is on by default, which hurts projects without any submodules (i.e. the
> former example above) a lot.

Right, I'll post a patch disabling that check completely when a project has
no submodules.

> In short, if "on demand" check is million times more expensive than
> actually doing it, the check does not have any value.
> 
> In the longer term, people who want to have the on-demand behaviour need
> to come up with a cheaper way to determine if it is necessary to recurse
> into submodules by fixing check_for_new_submodule_commits(), but until
> that happens, we should disable submodule recursion by default unless the
> user explicitly asks for it from the command line or from the configuration.

Ok, I'll see if I can come up with a solution for the performance regression.

^ permalink raw reply

* Re: [PATCH v3] date.c: Support iso8601 timezone formats
From: Junio C Hamano @ 2011-09-09 16:35 UTC (permalink / raw)
  To: Haitao Li; +Cc: git, Jeff King
In-Reply-To: <1315563033-9476-1-git-send-email-lihaitao@gmail.com>

Haitao Li <lihaitao@gmail.com> writes:

> Timezone designators including additional separator (`:`) are ignored.
> Actually zone designators in below formats are all valid according to
> ISO8601:2004, section 4.3.2:
>     [+-]hh, [+-]hhmm, [+-]hh:mm

Thanks for a re-roll.

> This patch teaches git recognizing zone designators with hours and
> minutes separated by colon, or minutes are empty.

The last sentence above makes it sound as if you are accepting

	"2011-09-17 12:34:56 +09:"

but I suspect that is not what you intend to allow.  Perhaps "we allowed
hh and hhmm and this teaches Git to recognize hh:mm format as well"?

> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> index f87abb5..5235b7a 100755
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -40,6 +40,11 @@ check_parse 2008-02 bad
>  check_parse 2008-02-14 bad
>  check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
>  check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'
> +check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'
> +check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 -0500'
> +check_parse '2008-02-14 20:30:45 -05' '2008-02-14 20:30:45 -0500'
> +check_parse '2008-02-14 20:30:45 -:30' '2008-02-14 20:30:45 +0000'
> +check_parse '2008-02-14 20:30:45 -05:00' '2008-02-14 20:30:45 -0500'
>  check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5

The above are from Peff, no?  We should credit him for tests in the
proposed log message.

Because the three formats 8601 specifies are "hh", "hhmm", or "hh:mm"
after +/-, among the above new tests, it appears to me that zone
designators "-5" and "-:30" should yield "bad", instead of being accepted.
The same for "+09:" I mentioned above, which is not in the new test.

^ permalink raw reply

* Re: [PATCH v3] date.c: Support iso8601 timezone formats
From: Junio C Hamano @ 2011-09-09 17:04 UTC (permalink / raw)
  To: Haitao Li; +Cc: git, Jeff King
In-Reply-To: <7vhb4lvflb.fsf@alter.siamese.dyndns.org>

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

> Haitao Li <lihaitao@gmail.com> writes:
>
>> Timezone designators including additional separator (`:`) are ignored.
>> Actually zone designators in below formats are all valid according to
>> ISO8601:2004, section 4.3.2:
>>     [+-]hh, [+-]hhmm, [+-]hh:mm
>
> Thanks for a re-roll.
>
>> This patch teaches git recognizing zone designators with hours and
>> minutes separated by colon, or minutes are empty.
>
> The last sentence above makes it sound as if you are accepting
>
> 	"2011-09-17 12:34:56 +09:"
>
> but I suspect that is not what you intend to allow.  Perhaps "we allowed
> hh and hhmm and this teaches Git to recognize hh:mm format as well"?

Also, I do not quite understand why the match_tz() logic needs to be that
long.

Wouldn't something like this patch (on top of yours) easier to follow?

 date.c |   50 +++++++++++++++++++++-----------------------------
 1 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/date.c b/date.c
index f8722c1..6079b1a 100644
--- a/date.c
+++ b/date.c
@@ -551,44 +551,36 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt
 
 static int match_tz(const char *date, int *offp)
 {
+	int min;
 	char *end;
-	int offset = strtoul(date+1, &end, 10);
-	int min, hour;
-	int n = end - date - 1;
+	int hour = strtoul(date + 1, &end, 10);
+	int n = end - (date + 1);
 
-	/*
-	 * ISO8601:2004(E) allows time zone designator been separated
-	 * by a clone in the extended format
-	 */
-	if (*end == ':') {
-		if (isdigit(end[1])) {
-			hour = offset;
-			min = strtoul(end+1, &end, 10);
-		} else {
-			/* Mark as invalid */
-			n = -1;
-		}
-	} else {
-		if (n == 1 || n == 2) {
-			/* Only hours specified */
-			hour = offset;
-			min = 0;
-		} else {
-			hour = offset / 100;
-			min = offset % 100;
-		}
+	if (n == 4) {
+		/* hhmm */
+		min = hour % 100;
+		hour = hour / 100;
+	} else if (n != 2) {
+		min = 99; /* random crap */
+	} else if (*end == ':') {
+		/* hh:mm? */
+		min = strtoul(end + 1, &end, 10);
+		if (end - (date + 1) != 5)
+			min = 99; /* random crap */
 	}
 
 	/*
-	 * Don't accept any random crap.. We might want to check that
-	 * the minutes are divisible by 15 or something too. (Offset of
+	 * Don't accept any random crap. Even though some places have
+	 * offset larger than 12 hours (e.g. Pacific/Kiritimati is at
+	 * UTC+14), there is something wrong if hour part is much
+	 * larger than that. We might also want to check that the
+	 * minutes are divisible by 15 or something too. (Offset of
 	 * Kathmandu, Nepal is UTC+5:45)
 	 */
-	if (n > 0 && min < 60) {
-		offset = hour*60+min;
+	if (min < 60 && hour < 24) {
+		int offset = hour * 60 + min;
 		if (*date == '-')
 			offset = -offset;
-
 		*offp = offset;
 	}
 	return end - date;

^ permalink raw reply related

* Re: [PATCH 2/2] push -s: skeleton
From: Junio C Hamano @ 2011-09-09 17:32 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Shawn O. Pearce
In-Reply-To: <20110909153441.GB28480@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Thu, Sep 08, 2011 at 03:19:54PM -0700, Junio C Hamano wrote:
>
>> My take on it is somewhat different. The only thing in the end result we
>> want to see is that the pushed commits are annotated with GPG signatures
>> in the notes tree, and there is no reason for us to cast in stone that
>> there has to be any significance in the commit history of the notes tree.
>
> Hmm. Is order really irrelevant? If you push a commit to master, moving
> it from X to Y, then push-rewind it back to X, then push a new commit Z,
> how do I cryptographically determine the correct final state of master?

You don't, as the certs are more about "up to this point, the pusher
trusts the history". I should have made it clearer in the cover letter to
the rerolled series, but the push certificate does not record the old
value of the ref in the reroll, because the point of signed-push is not
about signing the information that is equivalent to the server side
reflog.

You would have a signed push record that pushed Y, X and Z, and commit Z
sitting at the tip of 'master'. A few days may pass and then you run

    $ git log --show-notes=refs/notes/push-signature master

to find that the first commit with a push signature by somebody whose
judgement you trust is Z. Then you would need to inspect only commits that
are not ancestors of Z even if you suspect that some commits near the tip
of 'master' at the server side were tampered with.

You may at the same time find commits signed by the trusted people that
are meant for the same branch but are not contained in the history of
'master' (e.g. Y), which might indicate that the branch was rewound,
possibly by an intruder.

Another possible scenario. Later you and the pusher of Z may find that
when the pusher created Z, he merged something questionable and Z may now
have to be in "untrustable" set. You can dig further to find X at that
point.

> OK, I see. It is not "the server can do whatever it likes with the
> information" as much as "the server can do whatever it likes, but at the
> very least should eventually create a notes tree of a given form".

Yes, examples of things the server side might want to additionally do in
pre-receive-signature hook are to read the push certificate to implement
authorization (and it can be per-branch if you wanted to) and to forward
it immediately to offsite storage for safekeeping (the storage does not
have to use git notes to implement it).

^ permalink raw reply

* Re: RFD: leveraging GitHub's asciidoc rendering for our Documentation/
From: Junio C Hamano @ 2011-09-09 17:37 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <4E6A23DB.1040606@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Our own customisation is not loaded (of course) so that, e.g., the
> linkgit macro does not work; and the include statement makes GitHub's
> parser unhappy and choke.
>
> Does anybody feel this is worth pursuing?
>
> + Nicer blob view
> + Simpler way to judge documentation changes
> - Need to get our asciidoc config in there
> - GitHub's parser neeeds to learn include

Personally I am not very interested. Couldn't you just visit the html
branch instead for viewing?

^ permalink raw reply

* Re: git-p4.skipSubmitEdit
From: Luke Diamand @ 2011-09-09 17:47 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git, alevy
In-Reply-To: <loom.20110909T115356-849@post.gmane.org>

On 09/09/11 11:05, Vitor Antunes wrote:
> L. A. Linden Levy<alevy<at>  mobitv.com>  writes:
>
>>
>> Hi All,
>>
>> I have been using git-p4 for a while and it has allowed me to completely
>> change the way I develop and still be able to use perforce which my
>> company has for its main VCS. One thing that was driving me nuts was
>> that "git p4 submit" cycles through all of my individual commits and
>> asks me if I want to change them. The way I develop I often am checking
>> in 20 to 50 different small commits each with a descriptive git comment.
>> I felt like I was doing double duty by having emacs open on every commit
>> into perforce. So I modified git-p4 to have an option to skip the
>> editor. This option coupled with git-p4.skipSubmitEditCheck will make
>> the submission non-interactive for "git p4 submit".
>
> Hi Loren,
>
> This option was already included in a recent commit. The name that was
> used is "skipSubmitEditCheck". Please make sure you are using the most
> recent version of the script.

I put that option in - glad it's of use!

That option actually just skips the check of 'did the user edit the 
file'. git-p4 will still go ahead and bring up the file in the editor first.

I get around this myself by setting EDITOR=/bin/true. That works for me 
because I'm only using it in a script.

But it's possible that an additional option would actually be useful.



>
> But don't let this discourage you from submitting patches. Just makesure
> you clone git's repository and apply your patch over "maint" or "master"
> branches. For more details on how to submit patches you can read
> Documentation/SubmittingPatches.
>
> Vitor
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: git repository size / compression
From: Andreas Krey @ 2011-09-09 17:49 UTC (permalink / raw)
  To: John Szakmeister; +Cc: Carlos Martín Nieto, neubyr, git
In-Reply-To: <CAEBDL5U5-1nBGbWtb6+CfBrESoy8+p0Qqw1t1n_5EKFmpq9NhA@mail.gmail.com>

On Fri, 09 Sep 2011 12:05:03 +0000, John Szakmeister wrote:
...
> will at times choose to self-compress the file, instead of doing a
> delta and compressing.  IIRC, there is some heuristics in there for
> determining when to do that, but I forget the exact method.

Don't know about the compression part, but subversion does a delta of the nth
version of a file (not the global revision number n) against the version m, where
m is (n & (n-1)), or the least significant '1' bit flipped to '0'. That way, there
are only O(log(n)) instead of O(n) deltas to apply to get at a specific version.

[Was on the svn users list just then. They described it differently,
 but in essence it's that.]

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

^ permalink raw reply

* Re: git-p4.skipSubmitEdit
From: L. A. Linden Levy @ 2011-09-09 17:52 UTC (permalink / raw)
  To: Luke Diamand; +Cc: Vitor Antunes, git@vger.kernel.org
In-Reply-To: <4E6A514C.5080200@diamand.org>

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

I noticed that it only skipped the edit check. That is why I added the
skipSubmitEdit option. If they are both true then it never opens the
editor and never checks for an edit. Probably they should just be one
option. I think it should probably also be a command line option to skip
the editor.

- Alex

On Fri, 2011-09-09 at 13:47 -0400, Luke Diamand wrote:
> On 09/09/11 11:05, Vitor Antunes wrote:
> > L. A. Linden Levy<alevy<at>  mobitv.com>  writes:
> >
> >>
> >> Hi All,
> >>
> >> I have been using git-p4 for a while and it has allowed me to completely
> >> change the way I develop and still be able to use perforce which my
> >> company has for its main VCS. One thing that was driving me nuts was
> >> that "git p4 submit" cycles through all of my individual commits and
> >> asks me if I want to change them. The way I develop I often am checking
> >> in 20 to 50 different small commits each with a descriptive git comment.
> >> I felt like I was doing double duty by having emacs open on every commit
> >> into perforce. So I modified git-p4 to have an option to skip the
> >> editor. This option coupled with git-p4.skipSubmitEditCheck will make
> >> the submission non-interactive for "git p4 submit".
> >
> > Hi Loren,
> >
> > This option was already included in a recent commit. The name that was
> > used is "skipSubmitEditCheck". Please make sure you are using the most
> > recent version of the script.
> 
> I put that option in - glad it's of use!
> 
> That option actually just skips the check of 'did the user edit the 
> file'. git-p4 will still go ahead and bring up the file in the editor first.
> 
> I get around this myself by setting EDITOR=/bin/true. That works for me 
> because I'm only using it in a script.
> 
> But it's possible that an additional option would actually be useful.
> 
> 
> 
> >
> > But don't let this discourage you from submitting patches. Just makesure
> > you clone git's repository and apply your patch over "maint" or "master"
> > branches. For more details on how to submit patches you can read
> > Documentation/SubmittingPatches.
> >
> > Vitor
> >
> > --
> > 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
> 

-- 
Alex Linden Levy
Senior Software Engineer
MobiTV, Inc.
6425 Christie Avenue, 5th Floor, Emeryville, CA 94608
phone 510.450.5190 mobile 720.352.8394
email alevy@mobitv.com  web www.mobitv.com


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH 0/6] Improved infrastructure for refname normalization
From: Junio C Hamano @ 2011-09-09 17:57 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: gitzilla, git, cmn
In-Reply-To: <4E6A31D1.5020404@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> The library could do the normalization, but
>
> 1. It would probably cost a lot of redundant checks as reference names
> pass in and out of the library and back in again
>
> 2. Normalization requires copying or overwriting the incoming string, so
> each time a refname crosses the library perimeter there might have to be
> an extra memory allocation with the associated headaches of dealing with
> the ownership of the memory.
>
> 3. The library doesn't encapsulate all uses of reference names; for
> example, for_each_ref() invokes a callback function with the refname as
> an argument.  The callback function is free to do a strcmp() of the
> refname (normalized by the library) with some arbitrary string that it
> got from the command line.  Either the caller has to do the
> normalization itself (i.e., outside of the library) or the library has
> to learn how to do every possible filtering operation with refnames.

4. The caller needs to be corrected to pay attention to the normalization
the library did for it. Your code may use a string as a ref and then
create something based on the refname; illustrating with a fictitious
example:

	ref = make_branch_ref("refs/heads/%s", branch_name);
        update_ref(ref, sha1);
        write_log("created branch '%s'", branch_name);

Even though make_branch_ref() may have removed duplicated slashes from the
name in "branch_name" when it computed "ref", the log still will record
unnormalized name.

I think the callers need to be aware of the normalization in practice
anyway for this reason, and a good way forward is to give the callers a
library interface to do so. It might even make sense to make the other
parts of the API _reject_ unnormalized input to catch offending callers.

By the way, does this series introduce new infrastructure features that
can be reused in different areas, such as Hui's "alt_odb path
normalization" patch?

^ permalink raw reply

* [PATCH] fetch: skip on-demand checking when no submodules are configured
From: Jens Lehmann @ 2011-09-09 18:22 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git Mailing List, Martin Fick, Heiko Voigt, Michael Haggerty

It makes no sense to do the - possibly very expensive - call to "rev-list
<new-ref-sha1> --not --all" in check_for_new_submodule_commits() when
there aren't any submodules configured.

Leave check_for_new_submodule_commits() early when no name <-> path
mappings for submodules are found in the configuration. To make that work
reading the configuration had to be moved further up in cmd_fetch(), as
doing that after the actual fetch of the superproject was too late.

Reported-by: Martin Fick <mfick@codeaurora.org>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

This achieves the first goal: Don't let people pay a performance penalty
when they don't even use submodules. On Michael's test repo from [1] the
time for a full fetch went down from 142 seconds (current master) to one
second which is - not surprisingly - the same as using current master
with the --no-recurse-submodules option.

Now back to the drawing board to fix the performance regression for those
people who are using submodules ...

[1] http://comments.gmane.org/gmane.comp.version-control.git/177103

 builtin/fetch.c |   15 +++++++++------
 submodule.c     |    4 ++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 93c9938..e422ced 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -941,6 +941,15 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);

+	if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
+		if (recurse_submodules_default) {
+			int arg = parse_fetch_recurse_submodules_arg("--recurse-submodules-default", recurse_submodules_default);
+			set_config_fetch_recurse_submodules(arg);
+		}
+		gitmodules_config();
+		git_config(submodule_config, NULL);
+	}
+
 	if (all) {
 		if (argc == 1)
 			die(_("fetch --all does not take a repository argument"));
@@ -976,12 +985,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
 		const char *options[10];
 		int num_options = 0;
-		if (recurse_submodules_default) {
-			int arg = parse_fetch_recurse_submodules_arg("--recurse-submodules-default", recurse_submodules_default);
-			set_config_fetch_recurse_submodules(arg);
-		}
-		gitmodules_config();
-		git_config(submodule_config, NULL);
 		add_options_to_argv(&num_options, options);
 		result = fetch_populated_submodules(num_options, options,
 						    submodule_prefix,
diff --git a/submodule.c b/submodule.c
index 7a76edf..ad86534 100644
--- a/submodule.c
+++ b/submodule.c
@@ -481,6 +481,10 @@ void check_for_new_submodule_commits(unsigned char new_sha1[20])
 	const char *argv[] = {NULL, NULL, "--not", "--all", NULL};
 	int argc = ARRAY_SIZE(argv) - 1;

+	/* No need to check if there are no submodules configured */
+	if (!config_name_for_path.nr)
+		return;
+
 	init_revisions(&rev, NULL);
 	argv[1] = xstrdup(sha1_to_hex(new_sha1));
 	setup_revisions(argc, argv, &rev, NULL);
-- 
1.7.7.rc0.189.gf9175

^ permalink raw reply related

* Re: The imporantance of including http credential caching in 1.7.7
From: Jeff King @ 2011-09-09 18:27 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: John Szakmeister, Kyle Neath, git
In-Reply-To: <4E69C8DC.7060008@drmicha.warpmail.net>

On Fri, Sep 09, 2011 at 10:05:48AM +0200, Michael J Gruber wrote:

> > Agreed. Anything harder than ssh keys is right out the window, because
> > they're always the alternative these people could be using (but can't or
> > don't want to).
> 
> Sue, the question was: What is easy enough? I hoped that people would be
> using gpg to check signed tags, and that there might be a simple,
> convenient gnupg installer for Win and Mac which ties into the
> respective wallet systems or provides one they use already.

I suspect most people aren't checking signed tags. And even if they did
have gpg installed, most people aren't going to want a new password
wallet.  They're going to want integration with what they're already
using.

Which isn't to say that a gpg-based wallet is wrong, it's just that I
don't think it's filling the role that really needs filled. If you want
to make such a wallet helper, you're welcome to. But it doesn't
necessarily need to be a part of git core, and if it's not, then maybe
it's worth looking at the zillion other password wallet programs that
exist.

FWIW, I keep my passwords in a gpg-encrypted file and wrote a 10-line
shell script helper to do lookups for git. :)

> > We could make our own gpg-based password wallet system, but I think it's
> > a really bad idea, for two reasons:
> > 
> >   1. It's reinventing the wheel. Which is bad enough as it is, but is
> >      doubly bad with security-related code, because it's very easy to
> >      screw something up when you're writing a lot of new code.
> 
> So please let's not deploy credential-store...

I'm tempted to agree. But I also think it represents a nice lowest
common denominator. No hassle, no setup, but no security either. And
there are situations where that's appropriate (e.g., for unattended
cron operation, it's not much different than an unencrypted ssh key on
disk). My compromise was to put a big warning at the top of the
documentation. Maybe that's not enough, though.

And as far as reinventing the wheel with security code, I don't think
git-credential-store counts. It's not secure at all, so there's very
little to screw up. :)

> On 1.+2.: The idea/hope was to use an existing wallet system which
> people use for gnupg already to store their passphrase. If that is not
> used then my suggestion does not help much (the issue of widespread
> deployment), though it still is a secure version of credential-store for
> those who want a desktop-independent secure credential store.

Yeah, if there is an existing wallet system based around gpg, then
absolutely there should be a helper for it. But I don't know that there
is such a widely deployed system. And the helper for it doesn't need to
ship with git-core; anybody who uses their wallet system is free to
write and distribute the helper.

-Peff

^ permalink raw reply

* Re: The imporantance of including http credential caching in 1.7.7
From: Jeff King @ 2011-09-09 18:34 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Kyle Neath, git
In-Reply-To: <4E69C8F0.9070204@drmicha.warpmail.net>

On Fri, Sep 09, 2011 at 10:06:08AM +0200, Michael J Gruber wrote:

> > So I think the wheels have been turning on this for quite a while from
> > GitHub's perspective.
> 
> Thanks for clarifying. While it should make no difference for the
> acceptance of patches, it's great to see GitHub invest into scratching
> their Git itches, and thus contribute back. That's how open source works
> as a business model :)

Yes. I don't often enough mention how awesome GitHub is for funding me
and giving me a free hand to improve git. They're doing everything
right. So let me mention it here one more time. :)

> > In the meantime, the best thing we can do to push it forward is to write
> > helpers. I implemented some basic ones that should work anywhere, but
> > aren't as nice as integration with existing keychains. Some people are
> > working on Linux ones. The single best thing GitHub can do to push this
> > forward right now is to provide a well-written OS X Keychain helper, and
> > to provide feedback on whether git's end of the API is good enough.
> 
> ... and one for Git on Windows? It seems we're lacking both Win and OS X
> developers here.

I mentioned OS X because of Kyle's mention of the GitHub for Mac client.
But yes, I do think in the long term we want something similar on
Windows. GitHub recently hired a developer with some Windows experience;
I'll try to see if he's interested in writing a credential helper.

-Peff

^ permalink raw reply

* Re: can Git encrypt/decrypt .gpg on push/fetch?
From: Jeff King @ 2011-09-09 18:42 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Aneesh Bhasin, tzz, git
In-Reply-To: <4E6A165D.5010703@drmicha.warpmail.net>

On Fri, Sep 09, 2011 at 03:36:29PM +0200, Michael J Gruber wrote:

> A) Keep blobs and checkout encrypted
> - Use an editor which can encrypt/decrypt on the fly (e.g. vim)
> - Use "*.gpg diff=gpg" in your attributes and
> [diff "gpg"]
>         textconv = gpg -d
>   in your config to have cleartext diffs. Use cachetextconv with caution ;)

I use something like this for my password store, though I use:

  textconv = gpg -qd --no-tty

to keep things as clean as possible. Running gpg-agent is a must, of
course.

The wallet itself is just a gpg-encrypted YAML file, with a few scripts
grep within the hierarchy. I'm happy to share the code if anybody is
interested. I've also written firefox hooks to fill website form fields,
but that code is a little gross.

> B) Keep blobs encrypted, checkout decrypted
> - Use Use "*.gpg filter=gpg" in your attributes and
> [filter "gpg"]
> 	smudge = gpg -d
> 	clean = gpg -e -r yourgpgkey
>   in your config.
> 
> I use A on a regular basis. B is untested (but patterned after a similar
> gzip filter I use). You may or may not have better results with "gpg -ea".

Yeah, I think that would work but have never tried it either.

-Peff

^ 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