Re: What's cooking in git.git (Dec 2013, #03; Thu, 12)

2013-12-12 Thread Torsten Bögershausen

On 12/13/2013 01:57 AM, Junio C Hamano wrote:

[Cooking]

* fc/transport-helper-fixes (2013-12-09) 6 commits
  - remote-bzr: support the new 'force' option
  - test-hg.sh: tests are now expected to pass
  - transport-helper: check for 'forced update' message
  - transport-helper: add 'force' to 'export' helpers
  - transport-helper: don't update refs in dry-run
  - transport-helper: mismerge fix

  Updates transport-helper, fast-import and fast-export to allow the
  ref mapping and ref deletion in a way similar to the natively
  supported transports.

  Will merge to 'next'.

This breaks t5541, (at least on my systems, both Linux and Mac OS)

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Eric S. Raymond
Martin Langhoff :
> On Thu, Dec 12, 2013 at 6:04 PM, Eric S. Raymond  wrote:
> > I'm not sure what counts as a nonsensical branching point. I do know that
> > Keith left this rather cryptic note in a REAME:
> 
> Keith names exactly what we are talking about.

Oh, yeah, I figured that much out.  What I wasn't clear on was (a) whether
that's a complete description of "nonsensical branching point" or whether there
are other pathologies fundamentally *different* from that one.

I'm also not sure I have the end state of what cvs-fast-export does in that
case visualized correctly. When he says: "an entirely disjoint history will
be created containing the branch revisions and all parents back to the
root", I'm visualizing something like this:

  abcdefgh
  \
   +123---4

Suppose the root is a our pathological branch point is at d, then it
sounds like he's saying cvs-fast-export will produce a changeset DAG
that looks like this:

  ab'---c'---d'---efgh
   \
+b''---c''---d''1234

What I'm not clear on here is how b is related to b' and b'', c to c' and c'',
and d to d' and d''.  Which file changes go to which commit?  I shall have to
craft some broken RCS files to find out.

Have I explained that I'm building a test suite?  I intend to know exactly
what the tool does in these cases and document it.

> Between my earlier explanation and Keith's notes it should be clear to
> you. It is absolutely trivial in CVS to have an "inconsistent"
> checkout (for example, if you switch branch with the -l parameter
> disabling recursion, or if you accidentally switch branch in a
> subdirectory).

That last one sounds easy to fall into and nasty. 

> On that inconsistent checkout, nothing prevents you from tagging it,
> nor from creating a new branch.
> 
> An importer with a 'consistent tree mentality' will look at the
> files/revs involved in that tag (or branching point) and find no tree
> to match.
> 
> CVS repos with that crap exist. x11/xorg did (Jim Gettys challenged me
> to try importing it at an LCA, after the Bazaar NG folks passed on
> it). Mozilla did as well.
> 
> 
> IMHO it is a valid path to skip importing the tag/branch. As long as
> main dev work was in HEAD, things end up ok (which goes back to my
> flying fish notes).

The other way to handle it would be to translate the history as though every
branch of a file subset had been an attempt to branch eveything.
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Martin Langhoff
On Thu, Dec 12, 2013 at 6:04 PM, Eric S. Raymond  wrote:
> I'm not sure what counts as a nonsensical branching point. I do know that
> Keith left this rather cryptic note in a REAME:

Keith names exactly what we are talking about. At that time, Keith was
struggling with the old xorg cvs repo which these and quite a few
other nasties. I was also struggling with the mozilla cvs repo with
its own gremlins.

Between my earlier explanation and Keith's notes it should be clear to
you. It is absolutely trivial in CVS to have an "inconsistent"
checkout (for example, if you switch branch with the -l parameter
disabling recursion, or if you accidentally switch branch in a
subdirectory).

On that inconsistent checkout, nothing prevents you from tagging it,
nor from creating a new branch.

An importer with a 'consistent tree mentality' will look at the
files/revs involved in that tag (or branching point) and find no tree
to match.

CVS repos with that crap exist. x11/xorg did (Jim Gettys challenged me
to try importing it at an LCA, after the Bazaar NG folks passed on
it). Mozilla did as well.


IMHO it is a valid path to skip importing the tag/branch. As long as
main dev work was in HEAD, things end up ok (which goes back to my
flying fish notes).

cheers,



m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Dec 2013, #03; Thu, 12)

2013-12-12 Thread Duy Nguyen
On Fri, Dec 13, 2013 at 7:57 AM, Junio C Hamano  wrote:
> * nd/negative-pathspec (2013-12-06) 3 commits
>   (merged to 'next' on 2013-12-12 at 9f340c8)
>  + pathspec.c: support adding prefix magic to a pathspec with mnemonic magic
>  + Support pathspec magic :(exclude) and its short form :!
>  + glossary-content.txt: rephrase magic signature part
>
>  Introduce "negative pathspec" magic, to allow "git log . ':!dir'" to
>  tell us "I am interested in everything but 'dir' directory".

A bit off topic, but that command does not work as-is. We need '--' to
separate pathspec.

$ ./git log . :\!t
fatal: :!t: no such path in the working tree.
Use 'git  -- ...' to specify paths that do not exist locally.

Something to be improved later..
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] send-pack.c: mark a file-local function static

2013-12-12 Thread Duy Nguyen
On Fri, Dec 13, 2013 at 6:15 AM, Ramsay Jones
 wrote:
> BTW, I have not been following these patches, but I noticed that the
> 'remove_nonexistent_ours_in_pack()' function has no callers. (There are
> two commented out callers - but they seem to have *always* been commented
> out ;-) ). So, step 5 is no longer required? :-D

It's more of an optimization than a requirement, to avoid expensive
calculation later. Uncommenting is possible but I need to pass the
pack file name around a bit.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Dec 2013, #03; Thu, 12)

2013-12-12 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[New Topics]

* bc/doc-merge-no-op-revert (2013-12-09) 1 commit
 - Documentation: document pitfalls with 3-way merge

 Will merge to 'next'.


* cc/replace-object-info (2013-12-12) 10 commits
 - Documentation/git-replace: describe --format option
 - builtin/replace: unset read_replace_refs
 - t6050: add tests for listing with --format
 - builtin/replace: teach listing using short, medium or full formats
 - sha1_file: perform object replacement in sha1_object_info_extended()
 - t6050: show that git cat-file --batch fails with replace objects
 - sha1_object_info_extended(): add an "unsigned flags" parameter
 - sha1_file.c: add lookup_replace_object_extended() to pass flags
 - replace_object: don't check read_replace_refs twice
 - rename READ_SHA1_FILE_REPLACE flag to LOOKUP_REPLACE_OBJECT

 read_sha1_file() that is the workhorse to read the contents given
 an object name honoured object replacements, but there is no
 corresponding mechanism to sha1_object_info() that is used to
 obtain the metainfo (e.g. type & size) about the object, leading
 callers to weird inconsistencies.

 Will merge to 'next'.


* fc/completion (2013-12-09) 1 commit
 - completion: fix completion of certain aliases

 Will merge to 'next'.


* fc/remote-helper-fixes (2013-12-09) 4 commits
 - remote-hg: add tests for special filenames
 - remote-hg: fix 'shared path' path
 - remote-helpers: add extra safety checks
 - remote-hg: avoid buggy strftime()

 Will merge to 'next'.


* fc/trivial (2013-12-09) 4 commits
 - remote: fix status with branch...rebase=preserve
 - fetch: add missing documentation
 - t: trivial whitespace cleanups
 - abspath: trivial style fix

 Will merge to 'next'.


* jk/cat-file-regression-fix (2013-12-12) 2 commits
 - cat-file: handle --batch format with missing type/size
 - cat-file: pass expand_data to print_object_or_die

 Will merge to 'next'.


* jk/pull-rebase-using-fork-point (2013-12-10) 2 commits
 - rebase: use reflog to find common base with upstream
 - pull: use merge-base --fork-point when appropriate

 Will merge to 'next'.


* jk/rev-parse-double-dashes (2013-12-09) 2 commits
 - rev-parse: be more careful with munging arguments
 - rev-parse: correctly diagnose revision errors before "--"

 "git rev-parse  -- " did not implement the usual
 disambiguation rules the commands in the "git log" family used in
 the same way.

 Will merge to 'next'.


* mo/subtree-split-updates (2013-12-10) 3 commits
 - subtree: add --edit option
 - subtree: allow --squash and --message with push
 - subtree: support split --rejoin --squash

 Comments?


* nd/remove-opt-boolean (2013-12-09) 1 commit
 - parse-options: remove OPT_BOOLEAN

 Will merge to 'next'.


* nd/shallow-clone (2013-12-10) 28 commits
 - git-clone.txt: remove shallow clone limitations
 - prune: clean .git/shallow after pruning objects
 - clone: use git protocol for cloning shallow repo locally
 - send-pack: support pushing from a shallow clone via http
 - receive-pack: support pushing to a shallow clone via http
 - smart-http: support shallow fetch/clone
 - remote-curl: pass ref SHA-1 to fetch-pack as well
 - send-pack: support pushing to a shallow clone
 - receive-pack: allow pushes that update .git/shallow
 - connected.c: add new variant that runs with --shallow-file
 - add GIT_SHALLOW_FILE to propagate --shallow-file to subprocesses
 - receive/send-pack: support pushing from a shallow clone
 - receive-pack: reorder some code in unpack()
 - fetch: add --update-shallow to accept refs that update .git/shallow
 - upload-pack: make sure deepening preserves shallow roots
 - fetch: support fetching from a shallow repository
 - clone: support remote shallow repository
 - fetch-pack.h: one statement per bitfield declaration
 - fetch-pack.c: move shallow update code out of fetch_pack()
 - shallow.c: steps 6 and 7 to select new commits for .git/shallow
 - shallow.c: the 8 steps to select new commits for .git/shallow
 - shallow.c: extend setup_*_shallow() to accept extra shallow commits
 - connect.c: teach get_remote_heads to parse "shallow" lines
 - make the sender advertise shallow commits to the receiver
 - clone: prevent --reference to a shallow repository
 - send-pack: forbid pushing from a shallow repository
 - remote.h: replace struct extra_have_objects with struct sha1_array
 - transport.h: remove send_pack prototype, already defined in send-pack.h

 Fetching from a shallow-cloned repository used to be forbidden,
 primarily because the codepaths involved were not carefully vetted
 and we did not bother supporting such usage. This attempts to allow
 object transfer out of a shallow-cloned repositor

Re: [PATCH] contrib/git-credential-gnome-keyring.c: small stylistic cleanups

2013-12-12 Thread Junio C Hamano
John Szakmeister  writes:

> On Mon, Dec 9, 2013 at 1:06 PM, Junio C Hamano  wrote:
> [snip]
>>
>> I thought we cast without SP after the (typename), i.e.
>>
>> gpointer *data = (gpointer *)user_data;
>
> I've found a mixture of both in the code base, and the
> CodingGuidelines doesn't say either way.  I'm happy to switch the file
> to no SP after the typename if that's the project preference.

Somewhat arbitrary and unscientific, but between

git grep -e '[^f]([a-z_ ]* \*)[^ ]' -- \*.c | wc -l
422
$ git grep -e '[^f]([a-z_ ]* \*) ' -- \*.c | wc -l
233

I see that we favor "(struct blah *)apointer" over "(int *)
apointer".  Many hits in the latter grep come from compat/
that are borrowed pieces of code we tend not to style-fix.

The leading [^f] is crudely excludes "sizeof(typename *)"; it does
not change the resulting picture in a major way, though.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] send-pack.c: mark a file-local function static

2013-12-12 Thread Ramsay Jones

Commit f2c681cf ("send-pack: support pushing from a shallow clone
via http", 05-12-2013) adds the 'advertise_shallow_grafts_buf'
function as an external symbol. This symbol does not require
more than file visibility.

Noticed by sparse. ("'advertise_shallow_grafts_buf' was not declared.
Should it be static?")

Signed-off-by: Ramsay Jones 
---

Hi Duy,

If you need to re-roll the patches in your 'nd/shallow-clone' branch,
could you please squash this into your patch. Thanks!

BTW, I have not been following these patches, but I noticed that the
'remove_nonexistent_ours_in_pack()' function has no callers. (There are
two commented out callers - but they seem to have *always* been commented
out ;-) ). So, step 5 is no longer required? :-D

ATB,
Ramsay Jones

 send-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/send-pack.c b/send-pack.c
index 2a199cc..6129b0f 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -183,7 +183,7 @@ static int advertise_shallow_grafts_cb(const struct 
commit_graft *graft, void *c
return 0;
 }
 
-void advertise_shallow_grafts_buf(struct strbuf *sb)
+static void advertise_shallow_grafts_buf(struct strbuf *sb)
 {
if (!is_repository_shallow())
return;
-- 
1.8.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Eric S. Raymond
Martin Langhoff :
> On Thu, Dec 12, 2013 at 3:58 PM, Eric S. Raymond  wrote:
> >>  - regardless of commit ids, do you synthesize an artificial commit?
> >> How do you define parenthood for that artificial commit?
> >
> > Because tagging is never used to deduce changesets, the case does not arise.
> 
> So if a branch has a nonsensical branching point, or a tag is
> nonsensical, is it ignored and not imported?

I don't know what happens when identically-named tags point at changes that
resolve into two different commits.  I will figure that out and document it.

There's evidence, in the form of some code that is #ifdefed out, that 
Keith considered trying to make synthetic commits from tag cliques. But
abandoned the idea because he couldn't figure out how to assign such
cliques to a branch.

I'm not sure what counts as a nonsensical branching point. I do know that
Keith left this rather cryptic note in a REAME:

Disjoint branch resolution. Branches occurring in a subset of the
files are not correctly resolved; instead, an entirely disjoint
history will be created containing the branch revisions and all
parents back to the root. I'm not sure how to fix this; it seems
to implicitly assume there will be only a single place to attach as
branch parent, which may not be the case. In any case, the right
revision will have a superset of the revisions present in the
original branch parent; perhaps that will suffice.

-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Martin Langhoff
On Thu, Dec 12, 2013 at 3:58 PM, Eric S. Raymond  wrote:
>>  - regardless of commit ids, do you synthesize an artificial commit?
>> How do you define parenthood for that artificial commit?
>
> Because tagging is never used to deduce changesets, the case does not arise.

So if a branch has a nonsensical branching point, or a tag is
nonsensical, is it ignored and not imported?

curious,



m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 0/4] Show extra branch refs in gitweb

2013-12-12 Thread Junio C Hamano
Krzesimir Nowak  writes:

> First patch splits some code to a function.
>
> Second patch fixes validation functions to return either 0 or 1,
> instead of undef or passed $input.
>
> Third patch adds the extra-branch-feature and some documentation.
>
> Fourth patch adds some visual differentation of branches from
> non-standard ref directories.

Thanks; will queue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules

2013-12-12 Thread Junio C Hamano
Jens Lehmann  writes:

> Am 12.12.2013 02:16, schrieb Junio C Hamano:
>> "W. Trevor King"  writes:
>> 
>>> For
>>> safety, maybe the default `init` should copy *everything* into
>>> .git/config, after which users can remove stuff they'd like to
>>> delegate to .gitmodules.
>> 
>> Copying everything into config is "be unsafe and inconvenient by
>> default for everybody", isn't it?  Folks who want safety are forced
>> to inspect the resulting entries in their config file (which is more
>> inconvenent if you compare with the design where nothing is copied
>> and nothing dynamically defaults to what then-current .gitmodules
>> happens to contain).  Folks who trust those who update .gitmodules
>> for them are forced to update their config every time upstream
>> decides to use different settings in .gitmodules, because they have
>> stale values in their config that mask what are in .gitmodules.
>> 
>> I think the solution we want is to copy only minimum to the config
>> (and that "minimum" may turn out to be "nothing"), and to default
>> keys that are only absolutely safe to .gitmodules file.
>
> I agree and will prepare a patch for that.
>
> What about teaching "git submodule sync" the "--url", "--update",
> "--fetch", "--ignore", "--branch" and "--all" options to allow the
> user to copy the current settings he wants from .gitmodules to
> .git/config (but only safe values of course)?

An option per variable, which forms an unbounded set over time? From
the syntax point of view, "--copy-config=url,update,..."  probably
is a better option, but I think that misses the point.  Copying will
freeze the choice in stone.

Also, as long as the copying is deliberately done with such an
option, copying potentially "unsafe" ones is perfectly fine.

Reading and using what are not copied from the .gitmodules file _is_
a lot more severe security risk, so your "only safe ones, of course"
should apply more heavily on that side. In principle, by default, we
should use *nothing* from .gitmodules, and make exceptions on case
by case basis, allowing only the safe ones.

What is missing is a support for those like W. Trevor who trust what
are in .gitmodules, and want to use values from there for ones we do
not add to that default list of exceptions. They are not helped by
such an option to say "copy these keys from .gitmodules to my
config". They do not want to freeze values to what was in there at
one point. They want to just follow along, whatever values happen to
be set in the .gitmodules file of the day.

So I _think_ a better approach would be to let users say something
like:

[submodule "frotz"]
useInTreeSetting = update ignore

in their .git/config file in the repository of the top-level
project, to tell Git:

When 'submodule.frotz.update' or 'submodule.frotz.ignore' is
needed, please read from the .gitmodules file to grab the value
for that setting. I trust the project as a whole to set a
suitable value for me.

and copy almost nothing to .git/config file upon 'init' time.

If we were to go this route, I would envision that this new variable
would be a list of keys to additionally allow defaulting to the
values found in .gitmodules; if we hardcode 'branch', for example,
as one of the keys that we fallback to .gitmodules, and if the user
does *not* want to follow along to the project's recommendation,
the user can just set "submodule.frotz.branch = " in
the .git/config file, and there is no need for the useIntreeSetting
variable to support "Git by default may allow this variable 'branch'
to be read from .gitmodules but I do not like that".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Eric S. Raymond
Martin Langhoff :
> If someone creates a nonsensical tag or branch point, tagging files
> from different commits, how do you handle it?
> 
>  - without commit ids, does it affect your guesses?

No.  Tagging is never used to deduce changesets. Look:

/*
 * The heart of the merge operation; detect when two
 * commits are "the same"
 */
static bool
rev_commit_match (rev_commit *a, rev_commit *b)
{
/*
 * Versions of GNU CVS after 1.12 (2004) place a commitid in
 * each commit to track patch sets. Use it if present
 */
if (a->commitid && b->commitid)
return a->commitid == b->commitid;
if (a->commitid || b->commitid)
return false;
if (!commit_time_close (a->date, b->date))
return false;
if (a->log != b->log)
return false;
if (a->author != b->author)
return false;
return true;
}

>  - regardless of commit ids, do you synthesize an artificial commit?
> How do you define parenthood for that artificial commit?

Because tagging is never used to deduce changesets, the case does not arise.

I have added an item to my to-do: document what the tool does with
inconsistent tags.
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 2/2] diff: don't read index when --no-index is given

2013-12-12 Thread Junio C Hamano
Thomas Gummerer  writes:

> git diff --no-index ... currently reads the index, during setup, when
> calling gitmodules_config().  This results in worse performance when the
> index is not actually needed.  This patch avoids calling
> gitmodules_config() when the --no-index option is given.  The times for
> executing "git diff --no-index" in the WebKit repository are improved as
> follows:
>
> Test  HEAD~3HEAD
> --
> 4001.1: diff --no-index   0.24(0.15+0.09)   0.01(0.00+0.00) -95.8%
>
> An additional improvement of this patch is that "git diff --no-index" no
> longer breaks when the index file is corrupt, which makes it possible to
> use it for investigating the broken repository.
>
> To improve the possible usage as investigation tool for broken
> repositories, setup_git_directory_gently() is also not called when the
> --no-index option is given.
>
> Also add a test to guard against future breakages, and a performance
> test to show the improvements.
>
> Signed-off-by: Thomas Gummerer 
> ---

Looks reasonable.

Thanks.  Will queue.

>  builtin/diff.c|  7 +--
>  t/perf/p4001-diff-no-index.sh | 22 ++
>  t/t4053-diff-no-index.sh  | 15 +++
>  3 files changed, 42 insertions(+), 2 deletions(-)
>  create mode 100755 t/perf/p4001-diff-no-index.sh
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index da69e4a..ea1dd65 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -298,7 +298,9 @@ int cmd_diff(int argc, const char **argv, const char 
> *prefix)
>   break;
>   }
>  
> - prefix = setup_git_directory_gently(&nongit);
> + if (!no_index)
> + prefix = setup_git_directory_gently(&nongit);
> +
>   /*
>* Treat git diff with at least one path outside of the
>* repo the same as if the command would have been executed
> @@ -311,7 +313,8 @@ int cmd_diff(int argc, const char **argv, const char 
> *prefix)
>   !path_inside_repo(prefix, argv[i + 1]
>   no_index = DIFF_NO_INDEX_IMPLICIT;
>  
> - gitmodules_config();
> + if (!no_index)
> + gitmodules_config();
>   git_config(git_diff_ui_config, NULL);
>  
>   init_revisions(&rev, prefix);
> diff --git a/t/perf/p4001-diff-no-index.sh b/t/perf/p4001-diff-no-index.sh
> new file mode 100755
> index 000..683be69
> --- /dev/null
> +++ b/t/perf/p4001-diff-no-index.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +test_description="Test diff --no-index performance"
> +
> +. ./perf-lib.sh
> +
> +test_perf_large_repo
> +test_checkout_worktree
> +
> +file1=$(git ls-files | tail -n 2 | head -1)
> +file2=$(git ls-files | tail -n 1 | head -1)
> +
> +test_expect_success "empty files, so they take no time to diff" "
> + echo >$file1 &&
> + echo >$file2
> +"
> +
> +test_perf "diff --no-index" "
> + git diff --no-index $file1 $file2 >/dev/null
> +"
> +
> +test_done
> diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
> index 979e983..077c775 100755
> --- a/t/t4053-diff-no-index.sh
> +++ b/t/t4053-diff-no-index.sh
> @@ -29,4 +29,19 @@ test_expect_success 'git diff --no-index relative path 
> outside repo' '
>   )
>  '
>  
> +test_expect_success 'git diff --no-index with broken index' '
> + (
> + cd repo &&
> + echo broken >.git/index &&
> + git diff --no-index a ../non/git/a
> + )
> +'
> +
> +test_expect_success 'git diff outside repo with broken index' '
> + (
> + cd repo &&
> + git diff ../non/git/a ../non/git/b
> + )
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 00/10] teach replace objects to sha1_object_info_extended()

2013-12-12 Thread Junio C Hamano
Christian Couder  writes:

> Here is version 3 of a patch series to improve the way
> sha1_object_info_extended() behaves when it is passed a
> replaced object. The idea is to add a flags argument to it
> in the same way as what has been done to read_sha1_file().

Thanks.

Will take a look again (in the meantime, will queue on 'pu').

I do not think the new name for the bit is necessary nor it is a
good change, though.  Given an object name, reading the data and
inspecting the metadata (i.e. type and size) should yield consistent
results, so the original name is a perfectly appropriate name that
means "use the replacement object when using read-sha1-file and
friends to ask about an object".  The use of the replacement object
happens to be implmented via lookup-replace-object helper, but that
is an implementation detail of _how_ it is done, not the high level
descroption of _what_ the callers want to see done.

But that is just a minor nit.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Martin Langhoff
On Thu, Dec 12, 2013 at 2:39 PM, Eric S. Raymond  wrote:
> Yikes!  That is a much stricter stability criterion than I thought you
> were specifying.

:-) -- cvsps's approach is: if you have a cache, you can remember the
lies you told earlier.

It is impossible to be stable purely from the source data in the face
of these issues.

CVS is truly a PoS.

> I think it would handle 1a in a stable way

that is pretty important. Files added on a branch not affecting HEAD
and earlier branch checkout matters.


> What I think this means is that cvs-fast-export is stable if you are
> using a server/client combination that generates commitids (that is,
> GNU CVS of any version newer than 1.12 of 2004, or CVS-NT). It is
> *not* necessary for stability that the entire history have them.
>
> Here's how the logic works out:
>
> 1. Commits grouped by commitid are stable - nothing in CVS ever rewrites
> those or assigns a duplicate.
>
> 2. No file change made with a commitid can destabilize a commit guess
> made without them, because the similarity checker never tries to put both
> kinds in a single changeset.
>
> Can you detect any flaw in this?

If someone creates a nonsensical tag or branch point, tagging files
from different commits, how do you handle it?

 - without commit ids, does it affect your guesses?

 - regardless of commit ids, do you synthesize an artificial commit?
How do you define parenthood for that artificial commit?

curious,



m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Eric S. Raymond
Martin Langhoff :
> IIRC, making the output stable is nontrivial, specially on branches.
> Two cases are still in my mind, from when I was wrestling with cvsps.
> 
> 1 - For a history with CVS HEAD and a long-running "stable release"
> branch ("STABLE"), which branched at P1...
> 
>a - adding a file only at the tip of STABLE "retroactively changes
> history"  for P1 and perhaps CVS HEAD
> 
>b - forgetting to properly tag a subset of files with the branch
> tag, and doing it later retroactively changes history
> 
> 2 - you can create a new branch or tag with files that do not belong
> together in any "commit". Doing so changes history retroactively
> 
> ... when I say "changes history", I mean that the importers I know
> revise their guesses of what files were seen together in a 'commit'.
> This is specially true for history recorded with early cvs versions
> that did not record a 'commit id'.

Yikes!  That is a much stricter stability criterion than I thought you
were specifying.   No, cvs-fast-export probably doesn't satify all of these.
I think it would handle 1a in a stable way, but 1b and 2 would throw it.

I'm sure it can't be fooled in the presence of commitids, though,
because when it has those it doesn't try to do any similarity
matching.  And (this is the important point) it won't match any change
with a commit-id to any change without one.

What I think this means is that cvs-fast-export is stable if you are
using a server/client combination that generates commitids (that is,
GNU CVS of any version newer than 1.12 of 2004, or CVS-NT). It is
*not* necessary for stability that the entire history have them.

Here's how the logic works out:

1. Commits grouped by commitid are stable - nothing in CVS ever rewrites
those or assigns a duplicate.

2. No file change made with a commitid can destabilize a commit guess
made without them, because the similarity checker never tries to put both 
kinds in a single changeset.

Can you detect any flaw in this?
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Subtree: My Status

2013-12-12 Thread Junio C Hamano
Adam Spiers  writes:

> On Mon, Apr 22, 2013 at 09:18:46AM +0200, Jeremy Rosen wrote:
>> David Green wrote:
>> > Please remember that I don't consider myself a gatekeeper to git
>> > subtree.  In fact I could use some help reviewing and approving
>> > patches.
>> > If anyone thinks a patch looks good, let Junio know.  It's my
>> > responsibility to object to things, not your responsibility to wait
>> > for
>> 
>> I guess it's more or less everybody's responsability to review patches, but
>> it seems to me that for the actual gate-keeping, Junio considers you 
>> in charge of git-subtree... Maybe there is an organisational quirk to sort-
>> out here... Junio ?
>
> I can't see any resolution to this in the mail archives.  What's the
> process for getting subtree patches accepted?

Somebody take the ownership of the area, if David Green who earlier
volunteered to do so and worked on it needs help reviewing, helping
polishing and giving thumbs-up on the patches, perhaps?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules

2013-12-12 Thread Jonathan Nieder
Jens Lehmann wrote:
> Am 12.12.2013 02:16, schrieb Junio C Hamano:

>> I think the solution we want is to copy only minimum to the config
>> (and that "minimum" may turn out to be "nothing"), and to default
>> keys that are only absolutely safe to .gitmodules file.
>
> I agree and will prepare a patch for that.
>
> What about teaching "git submodule sync" the "--url", "--update",
> "--fetch", "--ignore", "--branch" and "--all" options to allow the
> user to copy the current settings he wants from .gitmodules to
> .git/config (but only safe values of course)?

Why would I want to copy settings to .git/config when they are safe
enough to already be used directly from .gitmodules and the value from
.gitmodules is the one chosen to make sense when working with the
current revision?

URLs are kind of special because generally the newest value is the
most meaningful one, even when looking back over old history.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules

2013-12-12 Thread W. Trevor King
On Thu, Dec 12, 2013 at 07:57:51PM +0100, Jens Lehmann wrote:
> Am 12.12.2013 02:16, schrieb Junio C Hamano:
> > I think the solution we want is to copy only minimum to the config
> > (and that "minimum" may turn out to be "nothing"), and to default
> > keys that are only absolutely safe to .gitmodules file.
> 
> I agree and will prepare a patch for that.
> 
> What about teaching "git submodule sync" the "--url", "--update",
> "--fetch", "--ignore", "--branch" and "--all" options to allow the
> user to copy the current settings he wants from .gitmodules to
> .git/config (but only safe values of course)? Trevor, would you be
> ok to issue another command to copy everything to .git/config?

Sounds good to me.  The more stuff I can leave in .gitmodules, the
happier I'll be ;).  My second step will be removing “unsafe” values
from .git/config where possible, and I'll trust myself to check
.gitmodules after pulls.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: I have end-of-lifed cvsps

2013-12-12 Thread Martin Langhoff
On Thu, Dec 12, 2013 at 1:29 PM, Eric S. Raymond  wrote:
> I am almost certain the output of cvs-fast-export is stable.  I
> believe the output of cvsps-3.x was, too.  Not sure about 2.x.

IIRC, making the output stable is nontrivial, specially on branches.
Two cases are still in my mind, from when I was wrestling with cvsps.

1 - For a history with CVS HEAD and a long-running "stable release"
branch ("STABLE"), which branched at P1...

   a - adding a file only at the tip of STABLE "retroactively changes
history"  for P1 and perhaps CVS HEAD

   b - forgetting to properly tag a subset of files with the branch
tag, and doing it later retroactively changes history

2 - you can create a new branch or tag with files that do not belong
together in any "commit". Doing so changes history retroactively

... when I say "changes history", I mean that the importers I know
revise their guesses of what files were seen together in a 'commit'.
This is specially true for history recorded with early cvs versions
that did not record a 'commit id'.

cvsps has the strange "feature" that it will cache its
assumptions/guesses, and continue incrementally from there. So if a
change in the CVS repo means that the old guess is now invalidated, it
continues the charade instead of forcing a complete rewrite of the git
history.

Maybe the current crop of tools have developed stronger magic than
what was available a few years ago... the task did seem impossible to
me.

cheers,




m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git-submodule.sh respects submodule.$name.update in .git/config but not .gitmodules

2013-12-12 Thread Jens Lehmann
Am 12.12.2013 02:16, schrieb Junio C Hamano:
> "W. Trevor King"  writes:
> 
>> For
>> safety, maybe the default `init` should copy *everything* into
>> .git/config, after which users can remove stuff they'd like to
>> delegate to .gitmodules.
> 
> Copying everything into config is "be unsafe and inconvenient by
> default for everybody", isn't it?  Folks who want safety are forced
> to inspect the resulting entries in their config file (which is more
> inconvenent if you compare with the design where nothing is copied
> and nothing dynamically defaults to what then-current .gitmodules
> happens to contain).  Folks who trust those who update .gitmodules
> for them are forced to update their config every time upstream
> decides to use different settings in .gitmodules, because they have
> stale values in their config that mask what are in .gitmodules.
> 
> I think the solution we want is to copy only minimum to the config
> (and that "minimum" may turn out to be "nothing"), and to default
> keys that are only absolutely safe to .gitmodules file.

I agree and will prepare a patch for that.

What about teaching "git submodule sync" the "--url", "--update",
"--fetch", "--ignore", "--branch" and "--all" options to allow the
user to copy the current settings he wants from .gitmodules to
.git/config (but only safe values of course)? Trevor, would you be
ok to issue another command to copy everything to .git/config?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Martin Langhoff
On Thu, Dec 12, 2013 at 1:15 PM, Eric S. Raymond  wrote:
> That terminology -- "flying fish" and "dovetail" -- is interesting, and
> I have not heard it before.  It might be woth putting in the Jargon File.
> Can you point me at examples of live usage?

The canonical reference would be
http://cvsbook.red-bean.com/cvsbook.html#Going%20Out%20On%20A%20Limb%20(How%20To%20Work%20With%20Branches%20And%20Survive)

just by being on the internet and widely referenced it has probably
eclipsed in google-juice examples of earlier usage. Karl Fogel may
remember where he got the names from.

cheers,



m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Eric S. Raymond
Martin Langhoff :
> In my prior work, the "better" CVS importers would not have stable
> output, so were not appropriate for incremental imports.

That is disturbing.  I would consider lack of stability a severe and
unacceptable failure mode in such a tool, if only because of the
difficulties it creates for proper regression testing.

If cvs-fast-export does not already have this property I will fix it 
so it does.  And document that fact.
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Subtree: My Status

2013-12-12 Thread Adam Spiers
On Mon, Apr 22, 2013 at 09:18:46AM +0200, Jeremy Rosen wrote:
> David Green wrote:
> > Please remember that I don't consider myself a gatekeeper to git
> > subtree.  In fact I could use some help reviewing and approving
> > patches.
> > If anyone thinks a patch looks good, let Junio know.  It's my
> > responsibility to object to things, not your responsibility to wait
> > for
> 
> I guess it's more or less everybody's responsability to review patches, but
> it seems to me that for the actual gate-keeping, Junio considers you 
> in charge of git-subtree... Maybe there is an organisational quirk to sort-
> out here... Junio ?

I can't see any resolution to this in the mail archives.  What's the
process for getting subtree patches accepted?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Eric S. Raymond
Andreas Krey :
> But anyway, the replacement question is a) how fast the cvs-fast-export is
> and b) whether its output is stable, that is, if the cvs repo C yields
> a git repo G, will then C with a few extra commits yield G' where every
> commit in G (as identified by its SHA1) is also in G', and G' additionally
> contains the new commits that were made to the CVS repo.
> 
> If that is the case you effectively have an incremental mode, except that
> it's not quite as fast.

I am almost certain the output of cvs-fast-export is stable.  I
believe the output of cvsps-3.x was, too.  Not sure about 2.x.

I wrote the output stages for both cvsps-3.x and cvs-fast-export, and
went to some effort to verify that they write streams in the same
"most natural" way - marks sequential from :1, blobs always witten as
late as possible, fileops in the same sort order the git tools emit,
etc.

I have added writing a regression test test to verify the stability
property to the TODO list. I will have this nailed down before the
next point release, in a few days.
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Eric S. Raymond
Martin Langhoff :
> On Wed, Dec 11, 2013 at 11:26 PM, Eric S. Raymond  wrote:
> > You'll have to remind me what you mean by "incremental" here. Possibly
> > it's something cvs-fast-export could support.
> 
> User can
> 
>  - run a cvs to git import at time T, resulting in repo G
>  - make commits to cvs repo
>  - run cvs to git import at time T1, pointed to G, and the import tool
> will only add the new commits found in cvs between T and T1.

No, cvs-fast-export doesn't do that. However, it is fast enough that
you can probably just rebuild the whole repo each time you want to
move content. 

When I did the conversion of groff recently I was getting rates of
about 150 commits a second - and it will be faster now, because I
found an expensive operation in the output stage I could optimize
out.

Now that you have reminded me of this, I remember implementing a -i
option for cvsps-3.0 that could be combined with a time restriction 
to output incremental dumps. It's likely I could do the same
thing for cvs-fast-import.

> The above examples assume that the CVS repos have used "flying fish"
> approach in the "interesting" (i.e.: recent) parts of their history.
> 
> [ Simplifying a bit for non-CVS-geeks -- flying fish is using CVS HEAD
> for your development, plus 'feature branches' that get landed, plus
> long-lived 'stable release' branches. Most CVS projects in modern
> times use flying fish, which is a lot like what the git project uses
> in its own repo, but tuned to CVS's strengths (interesting commits
> linearized in CVS HEAD).
> 
> Other approaches ('dovetail') tend to end up with unworkable messes
> given CVS's weaknesses. ]

That terminology -- "flying fish" and "dovetail" -- is interesting, and
I have not heard it before.  It might be woth putting in the Jargon File.
Can you point me at examples of live usage?
-- 
http://www.catb.org/~esr/";>Eric S. Raymond
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Martin Langhoff
On Thu, Dec 12, 2013 at 12:17 PM, Andreas Krey  wrote:
> But anyway, the replacement question is a) how fast the cvs-fast-export is
> and b) whether its output is stable

In my prior work, the "better" CVS importers would not have stable
output, so were not appropriate for incremental imports.

And even the fastest ones were very slow on large repos.

That is why I am asking the question.

> It won't magically disappear from your machine, and you have been warned. :-)

However, esr is making the case that git-cvsimport should stop using
cvsps. My questions are aimed at understanding whether this actually
results in proposing that an important feature is dropped.

Perhaps a better alternative is now available.


m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Andreas Krey
On Thu, 12 Dec 2013 08:42:25 +, Martin Langhoff wrote:
...
>  - run a cvs to git import at time T, resulting in repo G
>  - make commits to cvs repo
>  - run cvs to git import at time T1, pointed to G, and the import tool
> will only add the new commits found in cvs between T and T1.

I'm pretty sure that being given only G the incremental approach wouldn't
work - some extra state would be required.

But anyway, the replacement question is a) how fast the cvs-fast-export is
and b) whether its output is stable, that is, if the cvs repo C yields
a git repo G, will then C with a few extra commits yield G' where every
commit in G (as identified by its SHA1) is also in G', and G' additionally
contains the new commits that were made to the CVS repo.

If that is the case you effectively have an incremental mode, except that
it's not quite as fast.

At least that would be good enough for us - we ended up running a
filter-branch on the resulting history, and that takes some time anyway.

...
> The cvsimport+cvsps combo does a reasonable (though imperfect) job on
> 'flying fish' CVS histories _and that is what most projects evolved to
> use_. If other cvs import tools can handle crazy histories, hats off
> to them. But careful with knifing cvsps!

It won't magically disappear from your machine, and you have been warned. :-)

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: I have end-of-lifed cvsps

2013-12-12 Thread Martin Langhoff
On Wed, Dec 11, 2013 at 11:26 PM, Eric S. Raymond  wrote:
> You'll have to remind me what you mean by "incremental" here. Possibly
> it's something cvs-fast-export could support.

User can

 - run a cvs to git import at time T, resulting in repo G
 - make commits to cvs repo
 - run cvs to git import at time T1, pointed to G, and the import tool
will only add the new commits found in cvs between T and T1.

> But what I'm trying to tell you is that, even after I've done a dozen
> releases and fixed the worst problems I could find, cvsps is far too
> likely to mangle anything that passes through it.  The idea that you
> are preserving *anything* valuable by sticking with it is a mirage.

The bugs that lead to a mangled history are real. I acknowledge and
respect that.

However, with those limitations, the incremental feature has value in
many scenarios.

The two main ones are as follows:

 - A developer is tracking his/her own patches on top of a CVS-based
project with git. This is often done with git-svn for example. If
old/convoluted branches in the far past are mangled, this user won't
care; as long as HEAD->master and/or the current/recent branch are
consistent with reality, the tool fits a need.

 - A project plans to transition to git gradually. Experienced
developers who'd normally work on CVS HEAD start working on git (and
landing their work on CVS afterwards). Old/mangled branches and tags
are of little interest, the big value is CVS HEAD (which is linear)
and possibly recent release/stable branches. The history captured is
good enough for git blame/log/pickaxe along the "master" line. At
transition time the original CVS repo can be kept around in readonly
mode, so people can still checkout the exact contents of an old branch
or tag for example (assuming no destructive "surgery" was done in the
CVS repo).

The above examples assume that the CVS repos have used "flying fish"
approach in the "interesting" (i.e.: recent) parts of their history.

[ Simplifying a bit for non-CVS-geeks -- flying fish is using CVS HEAD
for your development, plus 'feature branches' that get landed, plus
long-lived 'stable release' branches. Most CVS projects in modern
times use flying fish, which is a lot like what the git project uses
in its own repo, but tuned to CVS's strengths (interesting commits
linearized in CVS HEAD).

Other approaches ('dovetail') tend to end up with unworkable messes
given CVS's weaknesses. ]

The cvsimport+cvsps combo does a reasonable (though imperfect) job on
'flying fish' CVS histories _and that is what most projects evolved to
use_. If other cvs import tools can handle crazy histories, hats off
to them. But careful with knifing cvsps!

cheers,



m
-- 
 martin.langh...@gmail.com
 -  ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 ~ http://docs.moodle.org/en/User:Martin_Langhoff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re: Publishing "filtered branch repositories" - workflow / recommendations?

2013-12-12 Thread Heiko Voigt
On Wed, Dec 11, 2013 at 03:16:24PM -0800, Junio C Hamano wrote:
> Jens Lehmann  writes:
> 
> >> I think this is closely related to Martin's list of wishes we
> >> earlier saw in the thread: remind the user to push necessary
> >> submodule tip before the top-level commit that needs that commit in
> >> the submodule is pushed out.  Giving projects a way to implement
> >> such a policy decision would be good, and having a default policy,
> >> if we can find one that would be reasonable for any submodule users,
> >> would be even better.  Would adding a generic pre-push hook at the
> >> top-level sufficient for that kind of thing, I have to wonder.
> >
> > That could call "git push --dry-run --recurse-submodules=check" to
> > deny the push if the submodule commit isn't on a remote branch.
> > that would only work for a single hardcoded remote, as the remote
> > itself does not seem to be passed to the pre-push hook.
> >
> > So me thinks adding a configuration option for the --recurse-submodule
> > option of push is the best way to achieve that. This could be set to
> > "check" ...
> 
> Yes, that uses only a single hard-coded decision, and making the
> branch name or remote name customizable is not enough, as you are
> still hardcoding "if ... isn't on" part. It is not far-fetched to
> imagine a project wants to add more restrictions to what commit in
> the submodule history can be bound to a tree of a published commit
> in the top-level project (e.g. "must be a tagged release point",
> "must be older at least by more than two weeks", "must be signed by
> one of these developers' keys", etc.).
> 
> So I am not yet convinced that a simple "option" that supplies a few
> parameters to a single hard-coded policy is sufficient in the long
> run.

Well, for the implementation of --recurse-submodules=check on push we
tried to be as dumb as possible and just try to find out whether the
commit is on any remote. The reason is that when someone works on a
branch and pushes it for demonstration/backup/before holiday the most
common mistake is that he forgets to push the submodule when pushing the
superproject. So "some remote" seems to be the most common denominator
for push here. At least its better than no check at all.

For extended rules that answer questions like: "When am I allowed to
merge in master?" we need some kind of workflow definition from which we
can deduct such rules. A pre-push hook would allow to define rules but
how about an approach that is easier for the user and can maybe guide
him to the correct workflow.

I find workflow guidance is a general problem in git and even though we
use hooks at $dayjob they are not always sufficient. E.g. we enforce an
"all commits have to be reviewed first before they enter master" policy
which is correct in most situations. But for more lightweight projects I
would like to loosen this rule. The problem relies in the automatic
distribution of hooks or options for them by project instead of by
installation.

One idea: We could distribute a default set of enabled hooks that
implement several typical workflow options which can then be enabled by
simply choosing one option. The configuration should be distributeable
via the project e.g. a .githookconfig or .gitworkflowconfig ?

The git tools could then adapt their defaults depending on such a
workflow definition. But it also means we would first have to collect
and define some typical workflows.

What do others think? There probably have been other ideas about
workflow definitions already, no?

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re: [RFC/WIP PATCH] implement reading of submodule .gitmodules configuration into cache

2013-12-12 Thread Heiko Voigt
On Mon, Dec 09, 2013 at 03:37:50PM -0800, Junio C Hamano wrote:
> > +void submodule_config_cache_free(struct submodule_config_cache *cache)
> > +{
> > +   /* NOTE: its important to iterate over the name hash here
> > +* since paths might have multiple entries */
> 
> Style (multi-line comments).

Will fix.

> This is interesting.  I wonder what the practical consequence is to
> have a single submodule bound to the top-level tree more than once.
> Updating from one of the working tree will make the other working
> tree out of sync because the ultimate location of the submodule
> directory pointed at by the two .git gitdirs can only have a single
> HEAD, be it detached or on a branch, and a single index.

To clarify, when writing this comment I was not thinking about the same
submodule with multiple paths in the same tree but rather with the same
name under different paths in different commits.

> Not that the decision to enforce that names are unique in the
> top-level .gitmodules, and follow that decision in this part of the
> code to be defensive (not rely on the "one submodule can be bound
> only once to a top-level tree"), but shouldn't such a configuration
> to have a single submodule bound to more than one place in the
> top-level tree be forbidden?

Yes IMO, that should be forbidden currently. I do not think we actually
prevent the user from doing so but it can not happen by accident since
we derive the initial name from the local path. Maybe we should be more
strict about that and put more guards in place to avoid such
configurations from entering the database.

> > +   for_each_hash(&cache->for_name, free_one_submodule_config, NULL);
> > +   free_hash(&cache->for_path);
> > +   free_hash(&cache->for_name);
> > +}
> > +
> > +static unsigned int hash_sha1_string(const unsigned char *sha1, const char 
> > *string)
> > +{
> > +   int c;
> > +   unsigned int hash, string_hash = 5381;
> > +   memcpy(&hash, sha1, sizeof(hash));
> > +
> > +   /* djb2 hash */
> > +   while ((c = *string++))
> > +   string_hash = ((string_hash << 5) + hash) + c; /* hash * 33 + c 
> > */
> 
> Hmm, the comment and the code does not seem to match in math here...

Yeah sorry that was a leftover from the code I started with. In the
beginning it was a pure string hash. Will remove both comments (since
its also not a pure djb2 hash anymore).

Cheers Heiko
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html