Re: [PATCH] drop support for experimental loose objects

2013-11-22 Thread Jeff King
On Thu, Nov 21, 2013 at 09:19:25PM +0100, Christian Couder wrote:

 Yeah, I think it might report wrong size in case of replaced objects
 for example.
 I looked at that following Junio's comment about the
 sha1_object_info() API, which,
 unlike read_sha1_file() API, does not interact with the replace mechanism:
 
 http://thread.gmane.org/gmane.comp.version-control.git/234023/
 
 I started to work on a patch about this but didn't take the time to
 finish and post it.

That seems kind of crazy. Would the fix be as simple as this:

diff --git a/sha1_file.c b/sha1_file.c
index 10676ba..a051d6c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2529,6 +2529,8 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi)
struct pack_entry e;
int rtype;
 
+   sha1 = lookup_replace_object(sha1);
+
co = find_cached_object(sha1);
if (co) {
if (oi-typep)

or do we need some way for callers to turn off replacement? I notice
that read_sha1_file has such a feature, but it is only used in one
place. I guess we would need to audit all the sha1_object_info callers.

-Peff
--
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


Please Can I trust You

2013-11-22 Thread Hartmann, Bahar
I Have A Project For Charity Which I cannot complete because Of My Illness. 
Please Contact Me On: hans-zi...@live.de
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Jeff King
On Thu, Nov 21, 2013 at 04:19:43PM -0800, Junio C Hamano wrote:

 * np/pack-v4 (2013-09-18) 90 commits
  . packv4-parse.c: add tree offset caching
  . t1050: replace one instance of show-index with verify-pack
  . index-pack, pack-objects: allow creating .idx v2 with .pack v4
  . unpack-objects: decode v4 trees
  . unpack-objects: allow to save processed bytes to a buffer
  - ...
 
  Nico and Duy advancing the eternal vaporware pack-v4.  This is here
  primarily for wider distribution of the preview edition.
 
  Temporarily ejected from 'pu', to try out jk/pack-bitmap, which
  this topic conflicts with.

I had a look at the conflicts. Textually, I do not think it is anything
too serious; it is mostly a case of adding unrelated lines in the same
spot. I am happy to help with resolving that if there is a need.

However, there may be semantic conflicts. The big one I can think of is
how packfile reuse interacts with various versions. We only do pack
reuse during a --stdout pack to a client. At this point, that means we
must be outputting packv2. If we have packv2 on disk, we are fine. If we
have packv4 on disk, I guess we simply need to disable reuse, which
should not be hard.

Once we start sending packv4 to clients, we'll have to reevaluate.
Probably we can just get away with turning off reuse when there is a
mismatch, though if my understanding of packv4 is correct, we could
still reuse packv2 entries. We can put that off until somebody works on
packv4-on-the-wire, though. :)

 * jk/pack-bitmap (2013-11-18) 22 commits
  - compat/mingw.h: Fix the MinGW and msvc builds
  - pack-bitmap: implement optional name_hash cache
  - t/perf: add tests for pack bitmaps
  - t: add basic bitmap functionality tests
  - count-objects: recognize .bitmap in garbage-checking
  - repack: consider bitmaps when performing repacks
  - repack: handle optional files created by pack-objects
  - repack: turn exts array into array-of-struct
  - repack: stop using magic number for ARRAY_SIZE(exts)
  - pack-objects: implement bitmap writing
  - rev-list: add bitmap mode to speed up object lists
  - pack-objects: use bitmaps when packing objects
  - pack-bitmap: add support for bitmap indexes
  - documentation: add documentation for the bitmap format
  - ewah: compressed bitmap implementation
  - compat: add endianness helpers
  - sha1_file: export `git_open_noatime`
  - revision: allow setting custom limiter function
  - pack-objects: factor out name_hash
  - pack-objects: refactor the packing list
  - revindex: export new APIs
  - sha1write: make buffer const-correct
 
  Borrows the bitmap index into packfiles from JGit to speed up
  enumeration of objects involved in a commit range without having to
  fully traverse the history.

Looks like you picked up my latest re-roll with Ramsay's fix on top.
There wasn't a lot of review on this past round (I'm not surprised; it's
a dauntingly large chunk to review).  I outlined a few possible open
issues in the cover letter, but I'd be happy to build those on top,
which I think will make review of them a lot easier.

Do we want to try this in 'next' post-1.8.5, or should I try to prod an
area expert like Shawn into doing another round of review?

-Peff
--
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] drop support for experimental loose objects

2013-11-22 Thread Christian Couder
On Fri, Nov 22, 2013 at 10:58 AM, Jeff King p...@peff.net wrote:
 On Thu, Nov 21, 2013 at 09:19:25PM +0100, Christian Couder wrote:

 Yeah, I think it might report wrong size in case of replaced objects
 for example.
 I looked at that following Junio's comment about the
 sha1_object_info() API, which,
 unlike read_sha1_file() API, does not interact with the replace mechanism:

 http://thread.gmane.org/gmane.comp.version-control.git/234023/

 I started to work on a patch about this but didn't take the time to
 finish and post it.

 That seems kind of crazy. Would the fix be as simple as this:

 diff --git a/sha1_file.c b/sha1_file.c
 index 10676ba..a051d6c 100644
 --- a/sha1_file.c
 +++ b/sha1_file.c
 @@ -2529,6 +2529,8 @@ int sha1_object_info_extended(const unsigned char 
 *sha1, struct object_info *oi)
 struct pack_entry e;
 int rtype;

 +   sha1 = lookup_replace_object(sha1);
 +
 co = find_cached_object(sha1);
 if (co) {
 if (oi-typep)

 or do we need some way for callers to turn off replacement? I notice
 that read_sha1_file has such a feature, but it is only used in one
 place.

Yeah, indeed, I asked myself such a question and that's why it is not
so simple unfortunately.

In sha1_file.c, there is:

void *read_sha1_file_extended(const unsigned char *sha1,
  enum object_type *type,
  unsigned long *size,
  unsigned flag)
{
void *data;
char *path;
const struct packed_git *p;
const unsigned char *repl = (flag  READ_SHA1_FILE_REPLACE)
? lookup_replace_object(sha1) : sha1;

errno = 0;
data = read_object(repl, type, size);
...

And in cache.h, there is:

#define READ_SHA1_FILE_REPLACE 1
static inline void *read_sha1_file(const unsigned char *sha1, enum
object_type *type, unsigned long *size)
{
return read_sha1_file_extended(sha1, type, size,
READ_SHA1_FILE_REPLACE);
}

So the READ_SHA1_FILE_REPLACE is a way to disable replacement at compile time.

But in my opinion if we want such a knob, we should use it when we set
the read_replace_refs global variable.
For example with something like this:

diff --git a/environment.c b/environment.c
index 0a15349..7c99af8 100644
--- a/environment.c
+++ b/environment.c
@@ -44,7 +44,7 @@ const char *editor_program;
 const char *askpass_program;
 const char *excludes_file;
 enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
-int read_replace_refs = 1; /* NEEDSWORK: rename to use_replace_refs */
+int read_replace_refs = READ_SHA1_FILE_REPLACE; /* NEEDSWORK: rename
to use_replace_refs */
 enum eol core_eol = EOL_UNSET;
 enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
 unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;

@Junio what would you think about such a change?

 I guess we would need to audit all the sha1_object_info callers.

Yeah but when I looked at them, there were not many that looked dangerous.

Thanks,
Christian.
--
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 issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
[+CC: Jens, the goto-guy for submodules]

Sergey Sharybin wrote:
 Namely, `git ls-files -m` will show addons as modified, regardless
 ignore=all configuration. In the same time `git diff-index --name-only
 HEAD --` will show no changes at all.

This happens because diff-index handles submodules explicitly (see
diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My
opinion is that this is a bug, and git ls-files needs to be taught to
handle submodules properly.

 This leads to issues with Arcanist (which is a Phabricator's tool) who
 considers addons as uncommited changes and either complains on this or
 just adds this to commits.

Does Arcanist use `git ls-files -m` to check?

 There're also some more issues which happens to our
 developers and which i can not quite reproduce.

Do try to track down the other issues and let us know.

 Sometimes it happens so git checkout to another branch yields about
 uncommited changes to addons and doesn't checkout to another branch.

I've seldom used submodules with branches, so I'll let others chime in.

Cheers.
--
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] drop support for experimental loose objects

2013-11-22 Thread Jeff King
On Fri, Nov 22, 2013 at 12:04:01PM +0100, Christian Couder wrote:

 In sha1_file.c, there is:
 
 void *read_sha1_file_extended(const unsigned char *sha1,
   enum object_type *type,
   unsigned long *size,
   unsigned flag)
 {
 void *data;
 char *path;
 const struct packed_git *p;
 const unsigned char *repl = (flag  READ_SHA1_FILE_REPLACE)
 ? lookup_replace_object(sha1) : sha1;
 
 errno = 0;
 data = read_object(repl, type, size);
 ...
 
 And in cache.h, there is:
 
 #define READ_SHA1_FILE_REPLACE 1
 static inline void *read_sha1_file(const unsigned char *sha1, enum
 object_type *type, unsigned long *size)
 {
 return read_sha1_file_extended(sha1, type, size,
 READ_SHA1_FILE_REPLACE);
 }
 
 So the READ_SHA1_FILE_REPLACE is a way to disable replacement at compile time.

Is it? I did not have the impression anyone would ever redefine
READ_SHA1_FILE_REPLACE at compile time, but that it was a flag that
individual callsites would pass to read_sha1_file_extended to tell them
whether they were interested in replacements. And the obvious reasons to
not be are:

  1. You are doing some operation which needs real objects, like fsck or
 generating a packfile.

  2. You have already resolved any replacements, and want to make sure
 you are getting the same object used elsewhere (e.g., because you
 already printed its size :) ).

The only site which calls read_sha1_file_extended directly and does not
pass the REPLACE flag is in streaming.c. And that looks to be a case of
(2), since we resolve the replacement at the start in open_istream().

I am kind of surprised we do not need to do so for (1) in places like
pack-objects.c. Most of that code does not use read_sha1_file,
preferring instead to find the individual pack entries (for reuse). But
there are some calls to read_sha1_file, and I wonder if there is a bug
lurking there.

-Peff
--
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 issues with submodules

2013-11-22 Thread Sergey Sharybin
Hey,

Answers are inlined.


On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:

 [+CC: Jens, the goto-guy for submodules]

 Sergey Sharybin wrote:
  Namely, `git ls-files -m` will show addons as modified, regardless
  ignore=all configuration. In the same time `git diff-index --name-only
  HEAD --` will show no changes at all.

 This happens because diff-index handles submodules explicitly (see
 diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My
 opinion is that this is a bug, and git ls-files needs to be taught to
 handle submodules properly.

Shall i fire report somewhere or it's being handled by the folks
reading this ML?

  This leads to issues with Arcanist (which is a Phabricator's tool) who
  considers addons as uncommited changes and either complains on this or
  just adds this to commits.

 Does Arcanist use `git ls-files -m` to check?

Yes, Arcanist uses `git ls-files -m` to check whether there're local
modifications. We might also contact phab developers asking to change
it to `git diff --name-only HEAD --`.  Is there a preferable way to
get list of modified files and are this command intended to output the
same results?

  There're also some more issues which happens to our
  developers and which i can not quite reproduce.

 Do try to track down the other issues and let us know.

I'm trying, but doesn't happen here on laptop yet. Will give it
another try (do have some ideas). Also directed our developers here
who experienced the issue and might give some details,

  Sometimes it happens so git checkout to another branch yields about
  uncommited changes to addons and doesn't checkout to another branch.

 I've seldom used submodules with branches, so I'll let others chime in.

Ok, thanks anyway :)

-- 
With best regards, Sergey Sharybin
--
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: submodule update and core.askpass

2013-11-22 Thread Dmitry Neverov
BTW, a workaround is to pass a path to askpass script in the GIT_ASKPASS
environment variable. 
Jens Lehmann jens.lehm...@web.de writes:

 Am 16.11.2013 22:42, schrieb Thomas Rast:
 Dmitry Neverov dmitry.neve...@jetbrains.com writes:
 
 git -c core.askpass=pass.sh clone main-repo
 cd main-repo
 git submodule init
 git submodule sync
 git -c core.askpass=pass.sh submodule update

 The last command asks for a password interactively. 

 I've run bisect, it seems like it started failing from commit
 be8779f7ac9a3be9aa783df008d59082f4054f67. I've checked: submodule update
 works fine in 1.8.5.rc2 with removed call to clear_local_git_env.
 
 Aside from GIT_CONFIG_PARAMETERS, which this needs ...

 Yes, if I understand GIT_CONFIG_PARAMETERS correctly we should not
 clean it as the user explicitly asked us to use that setting.

 ..., I wonder if we
 should also let other variables pass through.  For example, if the user
 went out of their way to set GIT_ALTERNATE_OBJECT_DIRECTORIES, shouldn't
 we also respect that?

 Hmm, I'm not so sure. Does the user really want the setting of
 GIT_ALTERNATE_OBJECT_DIRECTORIES to be honored inside the submodule
 too or would he want a different setting (including none)? I suspect
 different users would give different answers. And wouldn't a working
 GIT_CONFIG_PARAMETERS (or configuring the submodule after the initial
 clone) be the solution for that?

 The list of variables that is unset by clear_local_git_env is $(git
 rev-parse --local-env-vars), currently
 
   GIT_ALTERNATE_OBJECT_DIRECTORIES
   GIT_CONFIG
   GIT_CONFIG_PARAMETERS
   GIT_OBJECT_DIRECTORY
   GIT_DIR
   GIT_WORK_TREE
   GIT_IMPLICIT_WORK_TREE
   GIT_GRAFT_FILE
   GIT_INDEX_FILE
   GIT_NO_REPLACE_OBJECTS
   GIT_PREFIX



-- 
Dmitry Neverov
JetBrains
http://www.jetbrains.com
Develop with pleasure!
--
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 v6 05/10] git fetch-pack: Add --diag-url

2013-11-22 Thread SZEDER Gábor
Hi,

On Thu, Nov 21, 2013 at 09:40:48PM +0100, Torsten Bögershausen wrote:
 diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
 index d87ddf7..9136f2a 100755
 --- a/t/t5500-fetch-pack.sh
 +++ b/t/t5500-fetch-pack.sh
 @@ -531,5 +531,62 @@ test_expect_success 'shallow fetch with tags does not 
 break the repository' '
   git fsck
   )
  '
 +check_prot_path() {
 +  actual 

There is no need to truncate actual here, ...

 + (git fetch-pack --diag-url $1 21 1stdout) | grep -v host= actual 
 

... because it will be overwritten in this line anyway.

 + echo Diag: url=$1 expected 
 + echo Diag: protocol=$2 expected 
 + echo Diag: path=$3 expected 
 + test_cmp expected actual
 +}
 +
 +check_prot_host_path() {
 +  actual 
 + git fetch-pack --diag-url $1 2actual 

Likewise.


Best,
Gábor

--
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 issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Sergey Sharybin wrote:
 On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra
 artag...@gmail.com wrote:

 [+CC: Jens, the goto-guy for submodules]

 Sergey Sharybin wrote:
  Namely, `git ls-files -m` will show addons as modified, regardless
  ignore=all configuration. In the same time `git diff-index --name-only
  HEAD --` will show no changes at all.

 This happens because diff-index handles submodules explicitly (see
 diff-lib.c), while ls-files doesn't (see builtin/ls-files.c). My
 opinion is that this is a bug, and git ls-files needs to be taught to
 handle submodules properly.

 Shall i fire report somewhere or it's being handled by the folks
 reading this ML?

Bugs are reported and tackled on the list.

  This leads to issues with Arcanist (which is a Phabricator's tool) who
  considers addons as uncommited changes and either complains on this or
  just adds this to commits.

 Does Arcanist use `git ls-files -m` to check?

 Yes, Arcanist uses `git ls-files -m` to check whether there're local
 modifications. We might also contact phab developers asking to change
 it to `git diff --name-only HEAD --`.  Is there a preferable way to
 get list of modified files and are this command intended to output the
 same results?

I just checked it out: it uses `git ls-files -m` to get the list of
unstaged changes; `git diff --name-only HEAD --` will list staged
changes as well.
--
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] gitweb: Make showing branches configurable

2013-11-22 Thread Krzesimir Nowak
Running 'make GITWEB_WANTED_REFS=heads wip gitweb.cgi' will create a
gitweb CGI script showing branches that appear in refs/heads/ and in
refs/wip/. Might be useful for gerrit setups where user branches are
not stored under refs/heads/.

Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
---

Notes:
I'm actually not sure if all those changes are really necessary as I
was mostly targeting it for Gerrit use. Especially I mean the changes
in git_get_remotes_list, fill_remote_heads and print_page_nav. I tried
to make it as general as it gets, so there's nothing Gerrit specific.

 gitweb/Makefile|  4 ++-
 gitweb/gitweb.perl | 94 +++---
 2 files changed, 72 insertions(+), 26 deletions(-)

diff --git a/gitweb/Makefile b/gitweb/Makefile
index cd194d0..361dce9 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -38,6 +38,7 @@ GITWEB_SITE_HTML_HEAD_STRING =
 GITWEB_SITE_HEADER =
 GITWEB_SITE_FOOTER =
 HIGHLIGHT_BIN = highlight
+GITWEB_WANTED_REFS = heads
 
 # include user config
 -include ../config.mak.autogen
@@ -148,7 +149,8 @@ GITWEB_REPLACE = \
-e 
's|++GITWEB_SITE_HTML_HEAD_STRING++|$(GITWEB_SITE_HTML_HEAD_STRING)|g' \
-e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
-   -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
+   -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' \
+   -e 's|++GITWEB_WANTED_REFS++|$(GITWEB_WANTED_REFS)|g'
 
 GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68c77f6..8bc9e9a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -17,6 +17,7 @@ use Encode;
 use Fcntl ':mode';
 use File::Find qw();
 use File::Basename qw(basename);
+use List::Util qw(min);
 use Time::HiRes qw(gettimeofday tv_interval);
 binmode STDOUT, ':utf8';
 
@@ -122,6 +123,9 @@ our $logo_label = git homepage;
 # source of projects list
 our $projects_list = ++GITWEB_LIST++;
 
+# list of directories under refs/ we want to display as branches
+our @wanted_refs = qw{++GITWEB_WANTED_REFS++};
+
 # the width (in characters) of the projects list Description column
 our $projects_list_description_width = 25;
 
@@ -632,8 +636,19 @@ sub feature_avatar {
 sub check_head_link {
my ($dir) = @_;
my $headfile = $dir/HEAD;
-   return ((-e $headfile) ||
-   (-l $headfile  readlink($headfile) =~ /^refs\/heads\//));
+
+   if (-e $headfile) {
+   return 1;
+   }
+   if (-l $headfile) {
+   my $rl = readlink($headfile);
+
+   for my $ref (@wanted_refs) {
+   return 1 if $rl =~ /^refs\/$ref\//;
+   }
+   }
+
+   return 0;
 }
 
 sub check_export_ok {
@@ -2515,6 +2530,7 @@ sub format_snapshot_links {
 sub get_feed_info {
my $format = shift || 'Atom';
my %res = (action = lc($format));
+   my $matched_ref = 0;
 
# feed links are possible only for project views
return unless (defined $project);
@@ -2522,12 +2538,17 @@ sub get_feed_info {
# or don't have specific feed yet (so they should use generic)
return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);
 
-   my $branch;
-   # branches refs uses 'refs/heads/' prefix (fullname) to differentiate
-   # from tag links; this also makes possible to detect branch links
-   if ((defined $hash_base  $hash_base =~ m!^refs/heads/(.*)$!) ||
-   (defined $hash   $hash  =~ m!^refs/heads/(.*)$!)) {
-   $branch = $1;
+   my $branch = undef;
+   # branches refs uses 'refs/' + $wanted_refs[x] + '/' prefix
+   # (fullname) to differentiate from tag links; this also makes
+   # possible to detect branch links
+   for my $ref (@wanted_refs) {
+   if ((defined $hash_base  $hash_base =~ m!^refs/$ref/(.*)$!) ||
+   (defined $hash   $hash  =~ m!^refs/$ref/(.*)$!)) {
+   $branch = $1;
+   $matched_ref = $ref;
+   last;
+   }
}
# find log type for feed description (title)
my $type = 'log';
@@ -2540,7 +2561,7 @@ sub get_feed_info {
}
 
$res{-title} = $type;
-   $res{'hash'} = (defined $branch ? refs/heads/$branch : undef);
+   $res{'hash'} = (defined $branch ? refs/$matched_ref/$branch : undef);
$res{'file_name'} = $file_name;
 
return %res;
@@ -3184,24 +3205,43 @@ sub git_get_project_owner {
return $owner;
 }
 
-sub git_get_last_activity {
-   my ($path) = @_;
-   my $fd;
+sub git_get_last_activity_age {
+   my ($refs) = @_;
+   my $fd = -1;
 
-   $git_dir = $projectroot/$path;
open($fd, -|, git_cmd(), 'for-each-ref',
 '--format=%(committer)',
 '--sort=-committerdate',
 '--count=1',
-'refs/heads') or 

git submodule update needs to be at the toplevel of working tree, why?

2013-11-22 Thread Odin Hørthe Omdal
Hi,

I'm usually in a subfolder doing actual work. A very common problem I
have is wanting to do a submodule update, but git really hates that. And
I wonder why?

It wouldn't be hard to cd to the toplevel working directory, do the
update, and cd back. It's what I have to do manually every time now
already:


  $ git submodule update
  You need to run this command from the toplevel of the working tree.

  $ cd ../..

  $ git submodule update
  Submodule path 'myproject/libs/external-module': checked out
  '434fdf32a7add62...

  $ cd -


I see that it comes from git-sh-setup, so no rationale for this rather
weird and surprising behavior is given in the git-submodule file. :)

-- 
  Odin Hørthe Omdal
  odi...@opera.com
--
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 update needs to be at the toplevel of working tree, why?

2013-11-22 Thread John Keeping
On Fri, Nov 22, 2013 at 02:22:30PM +0100, Odin Hørthe Omdal wrote:
 I'm usually in a subfolder doing actual work. A very common problem I
 have is wanting to do a submodule update, but git really hates that. And
 I wonder why?
 
 It wouldn't be hard to cd to the toplevel working directory, do the
 update, and cd back. It's what I have to do manually every time now
 already:

This restriction was removed in Git 1.8.4.
--
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] drop support for experimental loose objects

2013-11-22 Thread Christian Couder
On Fri, Nov 22, 2013 at 12:24 PM, Jeff King p...@peff.net wrote:
 On Fri, Nov 22, 2013 at 12:04:01PM +0100, Christian Couder wrote:

 In sha1_file.c, there is:

 void *read_sha1_file_extended(const unsigned char *sha1,
   enum object_type *type,
   unsigned long *size,
   unsigned flag)
 {
 void *data;
 char *path;
 const struct packed_git *p;
 const unsigned char *repl = (flag  READ_SHA1_FILE_REPLACE)
 ? lookup_replace_object(sha1) : sha1;

 errno = 0;
 data = read_object(repl, type, size);
 ...

 And in cache.h, there is:

 #define READ_SHA1_FILE_REPLACE 1
 static inline void *read_sha1_file(const unsigned char *sha1, enum
 object_type *type, unsigned long *size)
 {
 return read_sha1_file_extended(sha1, type, size,
 READ_SHA1_FILE_REPLACE);
 }

 So the READ_SHA1_FILE_REPLACE is a way to disable replacement at compile 
 time.

 Is it? I did not have the impression anyone would ever redefine
 READ_SHA1_FILE_REPLACE at compile time, but that it was a flag that
 individual callsites would pass to read_sha1_file_extended to tell them
 whether they were interested in replacements. And the obvious reasons to
 not be are:

   1. You are doing some operation which needs real objects, like fsck or
  generating a packfile.

   2. You have already resolved any replacements, and want to make sure
  you are getting the same object used elsewhere (e.g., because you
  already printed its size :) ).

 The only site which calls read_sha1_file_extended directly and does not
 pass the REPLACE flag is in streaming.c. And that looks to be a case of
 (2), since we resolve the replacement at the start in open_istream().

Yeah, you are right. Sorry for overlooking this.

But anyway it looks redundant to me to have both this REPLACE flag and
the read_replace_refs global variable, so I think a proper solution
would involve some significant refactoring.

And if we decide to keep a REPLACE flag we might need to add one to
sha1_object_info_extended() too.

Thanks,
Christian.
--
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 update needs to be at the toplevel of working tree, why?

2013-11-22 Thread Odin Hørthe Omdal
On Fri, Nov 22, 2013, at 14:47, John Keeping wrote:
 On Fri, Nov 22, 2013 at 02:22:30PM +0100, Odin Hørthe Omdal wrote:
  I'm usually in a subfolder doing actual work. A very common problem I
  have is wanting to do a submodule update, but git really hates that. And
  I wonder why?
  
  It wouldn't be hard to cd to the toplevel working directory, do the
  update, and cd back. It's what I have to do manually every time now
  already:
 
 This restriction was removed in Git 1.8.4.

Oh, awesome!  My system git is 1.8.3.2.

I'll do a manual install then :)

Sorry for the noise. (I didn't find any earlier talk about it, but I
only used gmame's search, so it might have missed something)

-- 
  Odin Hørthe Omdal
  odi...@opera.com
--
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 issues with submodules

2013-11-22 Thread Jeff King
On Fri, Nov 22, 2013 at 06:38:47PM +0530, Ramkumar Ramachandra wrote:

  Does Arcanist use `git ls-files -m` to check?
 
  Yes, Arcanist uses `git ls-files -m` to check whether there're local
  modifications. We might also contact phab developers asking to change
  it to `git diff --name-only HEAD --`.  Is there a preferable way to
  get list of modified files and are this command intended to output the
  same results?
 
 I just checked it out: it uses `git ls-files -m` to get the list of
 unstaged changes; `git diff --name-only HEAD --` will list staged
 changes as well.

That diff command compares the working tree and HEAD; if you are trying
to match `ls-files -m`, you probably wanted just `git diff --name-only`
to compare the working tree and the index. Although in a script you'd
probably want to use the plumbing `git diff-files` instead.

-Peff
--
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 issues with submodules

2013-11-22 Thread Sergey Sharybin
Ramkumar, not actually sure what you mean?

For me `git diff --name-only HEAD --` ignores changes to submodules
hash changes. Also apparently it became a known TODO for phabricator
developers [1].

Jeff, kinda trying to match yes. Just don't want changes to submodules
hash to be included.

So, after all is it expected behavior of ls-files or not and if not
shall i report it as a separate thread? :)

[1] https://secure.phabricator.com/rARCe62b23e67deacc24469525cc5dea2b297a5073fb


On Fri, Nov 22, 2013 at 9:11 PM, Jeff King p...@peff.net wrote:
 On Fri, Nov 22, 2013 at 06:38:47PM +0530, Ramkumar Ramachandra wrote:

  Does Arcanist use `git ls-files -m` to check?
 
  Yes, Arcanist uses `git ls-files -m` to check whether there're local
  modifications. We might also contact phab developers asking to change
  it to `git diff --name-only HEAD --`.  Is there a preferable way to
  get list of modified files and are this command intended to output the
  same results?

 I just checked it out: it uses `git ls-files -m` to get the list of
 unstaged changes; `git diff --name-only HEAD --` will list staged
 changes as well.

 That diff command compares the working tree and HEAD; if you are trying
 to match `ls-files -m`, you probably wanted just `git diff --name-only`
 to compare the working tree and the index. Although in a script you'd
 probably want to use the plumbing `git diff-files` instead.

 -Peff



-- 
With best regards, Sergey Sharybin
--
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 issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Jeff King wrote:
 I just checked it out: it uses `git ls-files -m` to get the list of
 unstaged changes; `git diff --name-only HEAD --` will list staged
 changes as well.

 That diff command compares the working tree and HEAD; if you are trying
 to match `ls-files -m`, you probably wanted just `git diff --name-only`
 to compare the working tree and the index. Although in a script you'd
 probably want to use the plumbing `git diff-files` instead.

Thanks for that. It's probably not worth fixing ls-files; I'll patch
Arcanist to use diff-files instead.
--
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] drop support for experimental loose objects

2013-11-22 Thread Jeff King
On Fri, Nov 22, 2013 at 03:23:31PM +0100, Christian Couder wrote:

  The only site which calls read_sha1_file_extended directly and does not
  pass the REPLACE flag is in streaming.c. And that looks to be a case of
  (2), since we resolve the replacement at the start in open_istream().
 
 Yeah, you are right. Sorry for overlooking this.
 
 But anyway it looks redundant to me to have both this REPLACE flag and
 the read_replace_refs global variable, so I think a proper solution
 would involve some significant refactoring.

I don't think it is redundant. The global variable is about does the
whole operation want the replace feature turned on and the flag is
about does this particular callsite want the replace featured turned
on. We use the feature iff both are true.

We could implement the callsite flag by tweaking the global right before
the call to read_sha1_file, but then we would have to remember to turn
it back on afterwards. If this were a language with dynamic scopes like
Perl, that would be easy. But in C you have to remember to reset it in
all code paths. :)

In some cases it does make sense to turn the feature off for a whole
command (like pack-objects); using the global makes sense there. And
indeed, we seem to do it already in things like fsck, index-pack, etc.
So that answers my question of why I did not see more of case (1) in my
previous email: they do not need per-callsite disabling, because they do
it for the whole command.

 And if we decide to keep a REPLACE flag we might need to add one to
 sha1_object_info_extended() too.

Yes, but somebody needs to look at all of the callsites and decide which
form they want. :)

I did a brief skim, and the ones I noticed were:

  - several spots in index-pack, pack-objects, etc. But these are
already covered by unsetting read_replace_refs.

  - replace_object looks at both the original and new object to compare
their types (due to your recent patches); it would obviously want to
get the true type of the original object

  - When creating tags and trees, we care about the type of the object
(the former for the type line of the tag, the latter to set the
mode). What should they do with replace objects? As above, it is
probably insane to switch types, so it may not matter for practical
purposes.

  - istream_source in streaming.c would probably want to turn it off for
the same reason it uses read_sha1_file_extended

So I think most sites would be unaffected, but due to the second and
fourth item in my list above, we would need a flag for
sha1_object_info_extended.

-Peff
--
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 v2] Revamp git-cherry(1)

2013-11-22 Thread Thomas Rast
git-cherry(1)'s description section has never really managed to
explain to me what the command does.  It contains too much explanation
of the algorithm instead of simply saying what goals it achieves, and
too much terminology that we otherwise do not use (fork-point instead
of merge-base).

Try a much more concise approach: state what it finds out, why this is
neat, and how the output is formatted, in a few short paragraphs.  In
return, provide much longer examples of how it fits into a
format-patch/am based workflow, and how it compares to reading the
same from git-log.

Also carefully avoid using merge in a context where it does not mean
something that comes from git-merge(1).  Instead, say apply in an
attempt to further link to patch workflow concepts.

While there, also omit the language about _which_ upstream branch we
treat as the default.  I literally just learned that we support having
several, so let's not confuse new users here, especially considering
that git-config(1) does _not_ document this.

Prompted-by: a.hue...@commend.com on #git
Signed-off-by: Thomas Rast t...@thomasrast.ch
---

Junio C Hamano wrote:
  +EXAMPLES
  +
  +
  +git-cherry is frequently used in patch-based workflows (see
  +linkgit:gitworkflows[7]) to determine if a series of patches has been
  +applied by the upstream maintainer.  In such a workflow you might
  +create and send a topic branch like this (fill in appropriate
  +arguments for `...`):
 
 I think the ASCII art commit graph that shows topology which we lost
 by this patch gave a more intiutive sense of what a topic branch
 like this looked like than an incomplete skeleton of a command
 sequence that would be understood by those who already know how to
 work with multiple branches.  Perhaps we want both?

Perhaps like this?  I tried to tie in directly with what a user might
see from git-log.

This does push the ascii art rather far down in the manpage, but even
with a puny laptop display and a large font size the new EXAMPLES is
well on the first page of the manpage.  So the hope is that a
still-confused user would at least see that there are examples.


 Documentation/git-cherry.txt | 136 ---
 1 file changed, 103 insertions(+), 33 deletions(-)

diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index 2d0daae..6d14b3e 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt
@@ -3,7 +3,7 @@ git-cherry(1)
 
 NAME
 
-git-cherry - Find commits not merged upstream
+git-cherry - Find commits not applied in upstream
 
 SYNOPSIS
 
@@ -12,46 +12,26 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-The changeset (or diff) of each commit between the fork-point and head
-is compared against each commit between the fork-point and upstream.
-The diffs are compared after removing any whitespace and line numbers.
+Determine whether there are commits in `head..upstream` that are
+equivalent to those in the range `limit..head`.
 
-Every commit that doesn't exist in the upstream branch
-has its id (sha1) reported, prefixed by a symbol.  The ones that have
-equivalent change already
-in the upstream branch are prefixed with a minus (-) sign, and those
-that only exist in the head branch are prefixed with a plus (+) symbol:
-
-   __*__*__*__*__ upstream
-  /
-fork-point
-  \__+__+__-__+__+__-__+__ head
-
-
-If a limit has been given then the commits along the head branch up
-to and including limit are not reported:
-
-   __*__*__*__*__ upstream
-  /
-fork-point
-  \__*__*__limit__-__+__ head
-
-
-Because 'git cherry' compares the changeset rather than the commit id
-(sha1), you can use 'git cherry' to find out if a commit you made locally
-has been applied upstream under a different commit id.  For example,
-this will happen if you're feeding patches upstream via email rather
-than pushing or pulling commits directly.
+The equivalence test is based on the diff, after removing whitespace
+and line numbers.  git-cherry therefore detects when commits have been
+copied by means of linkgit:git-cherry-pick[1], linkgit:git-am[1] or
+linkgit:git-rebase[1].
 
+Outputs the SHA1 of every commit in `limit..head`, prefixed with
+`-` for commits that have an equivalent in upstream, and `+` for
+commits that do not.
 
 OPTIONS
 ---
 -v::
-   Verbose.
+   Show the commit subjects next to the SHA1s.
 
 upstream::
-   Upstream branch to compare against.
-   Defaults to the first tracked remote branch, if available.
+   Upstream branch to search for equivalent commits.
+   Defaults to the upstream branch of HEAD.
 
 head::
Working branch; defaults to HEAD.
@@ -59,6 +39,96 @@ OPTIONS
 limit::
Do not report commits up to (and including) limit.
 
+EXAMPLES
+
+
+Patch workflows
+~~~
+
+git-cherry is frequently used in patch-based workflows (see
+linkgit:gitworkflows[7]) to 

Re: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Sergey Sharybin wrote:
 Ramkumar, not actually sure what you mean?

 For me `git diff --name-only HEAD --` ignores changes to submodules
 hash changes.

`git diff --name-only HEAD --` compares the worktree to HEAD (listing
both staged and unstaged changes); we want `git diff --name-only --`
to compare the worktree to the index (listing only unstaged changes),
as Peff notes.

 Also apparently it became a known TODO for phabricator
 developers [1].

That was me :)

 So, after all is it expected behavior of ls-files or not and if not
 shall i report it as a separate thread? :)

Actually, I doubt it's worth fixing ls-files. Your problem should be
fixed when this is merged (hopefully in a few hours):

  https://github.com/facebook/arcanist/pull/121

Cheers.
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Thomas Rast
Jeff King p...@peff.net writes:

 * jk/pack-bitmap (2013-11-18) 22 commits
[...]
  Borrows the bitmap index into packfiles from JGit to speed up
  enumeration of objects involved in a commit range without having to
  fully traverse the history.

 Looks like you picked up my latest re-roll with Ramsay's fix on top.
 There wasn't a lot of review on this past round (I'm not surprised; it's
 a dauntingly large chunk to review).  I outlined a few possible open
 issues in the cover letter, but I'd be happy to build those on top,
 which I think will make review of them a lot easier.

 Do we want to try this in 'next' post-1.8.5, or should I try to prod an
 area expert like Shawn into doing another round of review?

Hmm, maybe I missed something, but AFAICS you (or Vicent) never acted on
or responded to my June reviews in this thread:

  http://thread.gmane.org/gmane.comp.version-control.git/228918

and again mentioned here, though I didn't point out all of them:

  http://thread.gmane.org/gmane.comp.version-control.git/236587/focus=236740

Granted, the way I verified this was checking whether you renamed
rlw_xor_run_bit() to something more fitting, so perhaps you just forgot
that one thing but did all the rest.

-- 
Thomas Rast
t...@thomasrast.ch
--
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 issues with submodules

2013-11-22 Thread Sergey Sharybin
Ah, didn't notice you're the author of that pull-request Ramkumar :)

So guess issue with arc can be considered solved now. But i'm still
collecting more details about how to manage to commit change addons
hash without arc command even (it happens to Campbell Barton really
often).

Will report back when we'll know something.

On Fri, Nov 22, 2013 at 10:35 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Sergey Sharybin wrote:
 Ramkumar, not actually sure what you mean?

 For me `git diff --name-only HEAD --` ignores changes to submodules
 hash changes.

 `git diff --name-only HEAD --` compares the worktree to HEAD (listing
 both staged and unstaged changes); we want `git diff --name-only --`
 to compare the worktree to the index (listing only unstaged changes),
 as Peff notes.

 Also apparently it became a known TODO for phabricator
 developers [1].

 That was me :)

 So, after all is it expected behavior of ls-files or not and if not
 shall i report it as a separate thread? :)

 Actually, I doubt it's worth fixing ls-files. Your problem should be
 fixed when this is merged (hopefully in a few hours):

   https://github.com/facebook/arcanist/pull/121

 Cheers.



-- 
With best regards, Sergey Sharybin
--
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] drop support for experimental loose objects

2013-11-22 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I guess we would need to audit all the sha1_object_info callers.

Yup; I agree that was the conclusion of Christian's thread.


--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Jeff King
On Fri, Nov 22, 2013 at 05:52:37PM +0100, Thomas Rast wrote:

  Looks like you picked up my latest re-roll with Ramsay's fix on top.
  There wasn't a lot of review on this past round (I'm not surprised; it's
  a dauntingly large chunk to review).  I outlined a few possible open
  issues in the cover letter, but I'd be happy to build those on top,
  which I think will make review of them a lot easier.
 
  Do we want to try this in 'next' post-1.8.5, or should I try to prod an
  area expert like Shawn into doing another round of review?
 
 Hmm, maybe I missed something, but AFAICS you (or Vicent) never acted on
 or responded to my June reviews in this thread:
 
   http://thread.gmane.org/gmane.comp.version-control.git/228918
 
 and again mentioned here, though I didn't point out all of them:
 
   http://thread.gmane.org/gmane.comp.version-control.git/236587/focus=236740

Sorry, I didn't respond directly to the email. Vicent did a pass for
style and documentation shortly after the initial series, and then I did
another pass in the most recent re-roll, adding a C fallback for the
gcc builtin. I thought that covered it, but:

 Granted, the way I verified this was checking whether you renamed
 rlw_xor_run_bit() to something more fitting, so perhaps you just forgot
 that one thing but did all the rest.

I didn't touch that. Vicent, did you have a comment on the name (it
really does look like it is a negation, and the only caller is
ewah_not).

-Peff
--
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] drop support for experimental loose objects

2013-11-22 Thread Joey Hess
Jeff King wrote:
  BTW, I've also seen git cat-file --batch report wrong sizes for objects,
 
 Hrm. For --batch, I'd think we would open the whole object and notice
 the corruption, even with the current code. But for --batch-check, we
 use sha1_object_info, and for an experimental object, we do not need
 to de-zlib the object at all.  So we end up reporting whatever crap we
 decipher from the garbage bytes.  My patch would fix that, as we would
 not incorrectly guess an object is experimental anymore.
 
 If you have specific cases that trigger even after my patch, I'd be
 interested to see them.

I was seeing it with --batch, not --batch-check. Probably only with the
old experimental loose object format. In one case, --batch reported a
size of 20k, and only output 1k of data. With the object file I sent
earlier, --batch reports a huge size, and fails trying to allocate the
memory for it before it can output anything.

I also have seen at least once a corrupt pack file that caused git to try
and allocate a absurd quantity of memory.

Unfortunately I do not currently have exemplars for these, although I
should be able to run a less robust version of my code and find them
again. ;) Will try to find time to do that.

BTW, the fuzzing code is here:
http://source.git-repair.branchable.com/?p=source.git;a=blob;f=Git/Destroyer.hs

-- 
see shy jo


signature.asc
Description: Digital signature


Re: [PATCH] gitweb: Make showing branches configurable

2013-11-22 Thread Junio C Hamano
Krzesimir Nowak krzesi...@endocode.com writes:

 Running 'make GITWEB_WANTED_REFS=heads wip gitweb.cgi' will create a
 gitweb CGI script showing branches that appear in refs/heads/ and in
 refs/wip/. Might be useful for gerrit setups where user branches are
 not stored under refs/heads/.

 Signed-off-by: Krzesimir Nowak krzesi...@endocode.com
 ---

 Notes:
 I'm actually not sure if all those changes are really necessary as I
 was mostly targeting it for Gerrit use. Especially I mean the changes
 in git_get_remotes_list, fill_remote_heads and print_page_nav. I tried
 to make it as general as it gets, so there's nothing Gerrit specific.

Thanks.

Two knee-jerk reactions after a quick scan.

 - You include heads for normal builds by hardcoded
   GITWEB_WANTED_REFS = heads but include tags unconditionally
   by having @ref_views = (tags, @wanted_refs) in the code.  Why?

 - Does this have be a compile-time decision?  It looks like this is
   something that can and should be made controllable with the
   normal gitweb configuration mechanism.


  gitweb/Makefile|  4 ++-
  gitweb/gitweb.perl | 94 
 +++---
  2 files changed, 72 insertions(+), 26 deletions(-)

 diff --git a/gitweb/Makefile b/gitweb/Makefile
 index cd194d0..361dce9 100644
 --- a/gitweb/Makefile
 +++ b/gitweb/Makefile
 @@ -38,6 +38,7 @@ GITWEB_SITE_HTML_HEAD_STRING =
  GITWEB_SITE_HEADER =
  GITWEB_SITE_FOOTER =
  HIGHLIGHT_BIN = highlight
 +GITWEB_WANTED_REFS = heads
  
  # include user config
  -include ../config.mak.autogen
 @@ -148,7 +149,8 @@ GITWEB_REPLACE = \
   -e 
 's|++GITWEB_SITE_HTML_HEAD_STRING++|$(GITWEB_SITE_HTML_HEAD_STRING)|g' \
   -e 's|++GITWEB_SITE_HEADER++|$(GITWEB_SITE_HEADER)|g' \
   -e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
 - -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
 + -e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g' \
 + -e 's|++GITWEB_WANTED_REFS++|$(GITWEB_WANTED_REFS)|g'
  
  GITWEB-BUILD-OPTIONS: FORCE
   @rm -f $@+
 diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
 index 68c77f6..8bc9e9a 100755
 --- a/gitweb/gitweb.perl
 +++ b/gitweb/gitweb.perl
 @@ -17,6 +17,7 @@ use Encode;
  use Fcntl ':mode';
  use File::Find qw();
  use File::Basename qw(basename);
 +use List::Util qw(min);
  use Time::HiRes qw(gettimeofday tv_interval);
  binmode STDOUT, ':utf8';
  
 @@ -122,6 +123,9 @@ our $logo_label = git homepage;
  # source of projects list
  our $projects_list = ++GITWEB_LIST++;
  
 +# list of directories under refs/ we want to display as branches
 +our @wanted_refs = qw{++GITWEB_WANTED_REFS++};
 +
  # the width (in characters) of the projects list Description column
  our $projects_list_description_width = 25;
  
 @@ -632,8 +636,19 @@ sub feature_avatar {
  sub check_head_link {
   my ($dir) = @_;
   my $headfile = $dir/HEAD;
 - return ((-e $headfile) ||
 - (-l $headfile  readlink($headfile) =~ /^refs\/heads\//));
 +
 + if (-e $headfile) {
 + return 1;
 + }
 + if (-l $headfile) {
 + my $rl = readlink($headfile);
 +
 + for my $ref (@wanted_refs) {
 + return 1 if $rl =~ /^refs\/$ref\//;
 + }
 + }
 +
 + return 0;
  }
  
  sub check_export_ok {
 @@ -2515,6 +2530,7 @@ sub format_snapshot_links {
  sub get_feed_info {
   my $format = shift || 'Atom';
   my %res = (action = lc($format));
 + my $matched_ref = 0;
  
   # feed links are possible only for project views
   return unless (defined $project);
 @@ -2522,12 +2538,17 @@ sub get_feed_info {
   # or don't have specific feed yet (so they should use generic)
   return if (!$action || $action =~ /^(?:tags|heads|forks|tag|search)$/x);
  
 - my $branch;
 - # branches refs uses 'refs/heads/' prefix (fullname) to differentiate
 - # from tag links; this also makes possible to detect branch links
 - if ((defined $hash_base  $hash_base =~ m!^refs/heads/(.*)$!) ||
 - (defined $hash   $hash  =~ m!^refs/heads/(.*)$!)) {
 - $branch = $1;
 + my $branch = undef;
 + # branches refs uses 'refs/' + $wanted_refs[x] + '/' prefix
 + # (fullname) to differentiate from tag links; this also makes
 + # possible to detect branch links
 + for my $ref (@wanted_refs) {
 + if ((defined $hash_base  $hash_base =~ m!^refs/$ref/(.*)$!) ||
 + (defined $hash   $hash  =~ m!^refs/$ref/(.*)$!)) {
 + $branch = $1;
 + $matched_ref = $ref;
 + last;
 + }
   }
   # find log type for feed description (title)
   my $type = 'log';
 @@ -2540,7 +2561,7 @@ sub get_feed_info {
   }
  
   $res{-title} = $type;
 - $res{'hash'} = (defined $branch ? refs/heads/$branch : undef);
 + $res{'hash'} = (defined $branch ? refs/$matched_ref/$branch : undef);
   $res{'file_name'} = 

Re: Git issues with submodules

2013-11-22 Thread Sergey Sharybin
Ok, got it now.

To reproduce the issue:

- Run git submodule update --recursive to make sure their SHA is
changed. Then `git add /path/to/changed submodule` or just `git add .`
- Modify any file from the parent repository
- Neither of `git status`, `git diff` and `git diff-files --name-only`
will show changes to a submodule, only changes to that file which was
changed in parent repo.
- Make a git commit. It will not list changes to submodule as wll.
- `git show HEAD` will show changes to both file from and parent
repository (which is expected) and will also show changes to the
submodule hash (which is unexpected i'd say).

On Fri, Nov 22, 2013 at 11:01 PM, Sergey Sharybin sergey@gmail.com wrote:
 Ah, didn't notice you're the author of that pull-request Ramkumar :)

 So guess issue with arc can be considered solved now. But i'm still
 collecting more details about how to manage to commit change addons
 hash without arc command even (it happens to Campbell Barton really
 often).

 Will report back when we'll know something.

 On Fri, Nov 22, 2013 at 10:35 PM, Ramkumar Ramachandra
 artag...@gmail.com wrote:
 Sergey Sharybin wrote:
 Ramkumar, not actually sure what you mean?

 For me `git diff --name-only HEAD --` ignores changes to submodules
 hash changes.

 `git diff --name-only HEAD --` compares the worktree to HEAD (listing
 both staged and unstaged changes); we want `git diff --name-only --`
 to compare the worktree to the index (listing only unstaged changes),
 as Peff notes.

 Also apparently it became a known TODO for phabricator
 developers [1].

 That was me :)

 So, after all is it expected behavior of ls-files or not and if not
 shall i report it as a separate thread? :)

 Actually, I doubt it's worth fixing ls-files. Your problem should be
 fixed when this is merged (hopefully in a few hours):

   https://github.com/facebook/arcanist/pull/121

 Cheers.



 --
 With best regards, Sergey Sharybin



-- 
With best regards, Sergey Sharybin
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Thomas Rast
Jeff King p...@peff.net writes:

 Hmm, maybe I missed something, but AFAICS you (or Vicent) never acted on
 or responded to my June reviews in this thread:
 
   http://thread.gmane.org/gmane.comp.version-control.git/228918
[...]
 Granted, the way I verified this was checking whether you renamed
 rlw_xor_run_bit() to something more fitting, so perhaps you just forgot
 that one thing but did all the rest.

 I didn't touch that. Vicent, did you have a comment on the name (it
 really does look like it is a negation, and the only caller is
 ewah_not).

Hmm, so it really was that one unlucky thing :-)

I don't have much to say on the area, but if you think it helps you I
can set aside some time RSN to review the second half of the series,
too.  Back in June I only looked at the first half.

-- 
Thomas Rast
t...@thomasrast.ch
--
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 issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Sergey Sharybin wrote:
 To reproduce the issue:

 - Run git submodule update --recursive to make sure their SHA is
 changed. Then `git add /path/to/changed submodule` or just `git add .`
 - Modify any file from the parent repository
 - Neither of `git status`, `git diff` and `git diff-files --name-only`
 will show changes to a submodule, only changes to that file which was
 changed in parent repo.
 - Make a git commit. It will not list changes to submodule as wll.
 - `git show HEAD` will show changes to both file from and parent
 repository (which is expected) and will also show changes to the
 submodule hash (which is unexpected i'd say).

Thanks Sergey; I can confirm that this is a bug. For some reason, the
`git add .` is adding the ignored submodule to the index. After that,

  $ git diff-index @

is not showing the ignored submodule. Let me see if I can dig through
this in greater detail.
--
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 v2] Revamp git-cherry(1)

2013-11-22 Thread Junio C Hamano
Thomas Rast t...@thomasrast.ch writes:

  NAME
  
 +git-cherry - Find commits not applied in upstream
  
 +Determine whether there are commits in `head..upstream` that are
 +equivalent to those in the range `limit..head`.
  
 +The equivalence test is based on the diff, after removing whitespace
 +and line numbers.  git-cherry therefore detects when commits have been
 +copied by means of linkgit:git-cherry-pick[1], linkgit:git-am[1] or
 +linkgit:git-rebase[1].
  
 +Outputs the SHA1 of every commit in `limit..head`, prefixed with
 +`-` for commits that have an equivalent in upstream, and `+` for
 +commits that do not.

Thanks, this reads really much better than tha original.

We are listing those that need to be added to the upstream with +,
while listing those that can be dropped from yours if you rebase
with -.  Hinting the rationale behind the choice of +/-
somewhere may help as a mnemonic to the readers (see below).

 +EXAMPLES
 +
 +
 +Patch workflows
 +~~~
 +
 +git-cherry is frequently used in patch-based workflows (see
 +linkgit:gitworkflows[7]) to determine if a series of patches has been
 +applied by the upstream maintainer.  In such a workflow you might
 +create and send a topic branch like this:
 +
 +
 +$ git checkout -b topic origin/master
 +# work and create some commits
 +$ git format-patch origin/master
 +$ git send-email ... 00*
 +
 +Later, you can see whether your changes have been applied by saying
 +(still on `topic`):

Perhaps we want a blank line before Later, ... to be consistent
with all the other displayed examples here (I'll squash it locally
before queuing), even though AsciiDoc seems to format this just
fine.

 +
 +
 +$ git fetch  # update your notion of origin/master
 +$ git cherry -v
 +
 +
 +Concrete example
 +

A concrete example, perhaps?  I dunno.

 +In a situation where topic consisted of three commits, and the
 +maintainer applied two of them, the situation might look like:
 +
 +
 +$ git log --graph --oneline --decorate --boundary origin/master...topic
 +* 7654321 (origin/master) upstream tip commit
 +[... snip some other commits ...]
 +* 111 cherry-pick of C
 +* 111 cherry-pick of A
 +[... snip a lot more that has happened ...]
 +| * 000 (topic) commit C
 +| * 000 commit B
 +| * 000 commit A
 +|/
 +o 1234567 branch point
 +
 +
 +In such cases, git-cherry shows a concise summary of what has been
 +applied:

It shows a concise summary of what has yet to be applied (to be
consistent with the one-line description in the NAME section).

 +
 +$ git cherry origin/master topic
 +- 000... commit C
 ++ 000... commit B
 +- 000... commit A
 +

And the earlier why +/- could be done after this picture,
perhaps like:

Here, we see that the commits A and C (marked with `-`) can
be dropped from your `topic` branch when you rebase it on
top of `origin/master`, while the commit B (marked with `+`)
still needs to be kept so that it will be sent to be applied
to `origin/master`.

or somesuch?
--
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 v2] Revamp git-cherry(1)

2013-11-22 Thread Thomas Rast
Junio C Hamano gits...@pobox.com writes:

 We are listing those that need to be added to the upstream with +,
 while listing those that can be dropped from yours if you rebase
 with -.  Hinting the rationale behind the choice of +/-
 somewhere may help as a mnemonic to the readers (see below).
[...]
 And the earlier why +/- could be done after this picture,
 perhaps like:

   Here, we see that the commits A and C (marked with `-`) can
   be dropped from your `topic` branch when you rebase it on
   top of `origin/master`, while the commit B (marked with `+`)
   still needs to be kept so that it will be sent to be applied
   to `origin/master`.

 or somesuch?

Good idea, thanks.  Will integrate this more what still needs to be
integrated-minded wording into a v3.

-- 
Thomas Rast
t...@thomasrast.ch
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Looks like you picked up my latest re-roll with Ramsay's fix on top.
 There wasn't a lot of review on this past round (I'm not surprised; it's
 a dauntingly large chunk to review).  I outlined a few possible open
 issues in the cover letter, but I'd be happy to build those on top,
 which I think will make review of them a lot easier.

 Do we want to try this in 'next' post-1.8.5, or should I try to prod an
 area expert like Shawn into doing another round of review?

Yes ;-).

I recall starting to read the series over and then got sidetracked
in the middle and never finishing.  I'll try to make time sometime
this weekend (we are still buried in boxes after the move, though,
so no promises) myself.

How close is this what you guys are running in production these
days, by the way?
--
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 v2] Revamp git-cherry(1)

2013-11-22 Thread Junio C Hamano
Thomas Rast t...@thomasrast.ch writes:

 Junio C Hamano gits...@pobox.com writes:

 We are listing those that need to be added to the upstream with +,
 while listing those that can be dropped from yours if you rebase
 with -.  Hinting the rationale behind the choice of +/-
 somewhere may help as a mnemonic to the readers (see below).
 [...]
 And the earlier why +/- could be done after this picture,
 perhaps like:

  Here, we see that the commits A and C (marked with `-`) can
  be dropped from your `topic` branch when you rebase it on
  top of `origin/master`, while the commit B (marked with `+`)
  still needs to be kept so that it will be sent to be applied
  to `origin/master`.

 or somesuch?

 Good idea, thanks.  Will integrate this more what still needs to be
 integrated-minded wording into a v3.

Just to possibly save one round-trip, here is what I tentatively
queued on top of yours.

 Documentation/git-cherry.txt | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-cherry.txt b/Documentation/git-cherry.txt
index 6d14b3e..0ea921a 100644
--- a/Documentation/git-cherry.txt
+++ b/Documentation/git-cherry.txt
@@ -3,7 +3,7 @@ git-cherry(1)
 
 NAME
 
-git-cherry - Find commits not applied in upstream
+git-cherry - Find commits yet to be applied to upstream
 
 SYNOPSIS
 
@@ -56,6 +56,7 @@ $ git checkout -b topic origin/master
 $ git format-patch origin/master
 $ git send-email ... 00*
 
+
 Later, you can see whether your changes have been applied by saying
 (still on `topic`):
 
@@ -84,8 +85,8 @@ $ git log --graph --oneline --decorate --boundary 
origin/master...topic
 o 1234567 branch point
 
 
-In such cases, git-cherry shows a concise summary of what has been
-applied:
+In such cases, git-cherry shows a concise summary of what has yet to
+be applied:
 
 
 $ git cherry origin/master topic
@@ -94,6 +95,12 @@ $ git cherry origin/master topic
 - 000... commit A
 
 
+Here, we see that the commits A and C (marked with `-`) can be
+dropped from your `topic` branch when you rebase it on top of
+`origin/master`, while the commit B (marked with `+`) still needs to
+be kept so that it will be sent to be applied to `origin/master`.
+
+
 Using a limit
 ~
 
-- 
1.8.5-rc3-362-gdf10213

--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Vicent Marti
On Fri, Nov 22, 2013 at 6:26 PM, Jeff King p...@peff.net wrote:
 Granted, the way I verified this was checking whether you renamed
 rlw_xor_run_bit() to something more fitting, so perhaps you just forgot
 that one thing but did all the rest.

 I didn't touch that. Vicent, did you have a comment on the name (it
 really does look like it is a negation, and the only caller is
 ewah_not).

Yes, the name was ported straight from the original library and kept
as-is to make the translation more straightforward. These sources are
--again-- a translation, so I tried to remain as close to the original
Java implementation as possible.

I agree the name is not ideal, but it does make quite a bit of sense.
It effectively inverts the word based on the run bit, which is the
equivalent of xoring it with the bit if it's one.

Love,
vmg
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Vicent Martí
On Fri, Nov 22, 2013 at 8:36 PM, Junio C Hamano gits...@pobox.com wrote:
 Do we want to try this in 'next' post-1.8.5, or should I try to prod an
 area expert like Shawn into doing another round of review?

 Yes ;-).

 I recall starting to read the series over and then got sidetracked
 in the middle and never finishing.  I'll try to make time sometime
 this weekend (we are still buried in boxes after the move, though,
 so no promises) myself.

 How close is this what you guys are running in production these
 days, by the way?

We are running a slightly older version of the patchset, because we're
still on 1.8.4 and the current reroll doesn't apply cleanly there.

If this could make it to `next` some time next week, that would work
out great for us, because we may start considering using `next` as a
partial or full deployment on our production machines

This also means that we could exercise the patchset and everything
else that is queued up in next release... You must remember all the
corner cases and bugs peff brings to the list every time we deploy a
new Git to production. Wouldn't it be nice to have a thorough checking
of the current iteration *before* the release, and not after? :)

*hint hint*
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Junio C Hamano
Vicent Martí tan...@gmail.com writes:

 On Fri, Nov 22, 2013 at 8:36 PM, Junio C Hamano gits...@pobox.com wrote:

 We are running a slightly older version of the patchset, because we're
 still on 1.8.4 and the current reroll doesn't apply cleanly there.

 If this could make it to `next` some time next week, that would work
 out great for us, because we may start considering using `next` as a
 partial or full deployment on our production machines

I do not think potentially incompatible stuff that are slated for
2.0 that have been cooking in 'next' affects the server side, so
that may be a good and safe move.

 This also means that we could exercise the patchset and everything
 else that is queued up in next release...

There is no 'next release' though; there is no guarantee what is
cooking in 'next' will be in any future release ;-).

In any case, it is nice to see that people from a large hosting site
finally taking a hint from my occasional light complaints that come
after thanks for reporting whenever I see regression and breakage
report soon after a topic graduates to 'master' ;-).  It is good to
see that more people starting to adopt the 'next' branch early for
wider testing.

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


Re: What's cooking in git.git (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Thomas Rast
Vicent Marti vic...@github.com writes:

 On Fri, Nov 22, 2013 at 6:26 PM, Jeff King p...@peff.net wrote:
 I didn't touch that. Vicent, did you have a comment on the name (it
 really does look like it is a negation, and the only caller is
 ewah_not).

 Yes, the name was ported straight from the original library and kept
 as-is to make the translation more straightforward. These sources are
 --again-- a translation, so I tried to remain as close to the original
 Java implementation as possible.

 I agree the name is not ideal, but it does make quite a bit of sense.
 It effectively inverts the word based on the run bit, which is the
 equivalent of xoring it with the bit if it's one.

It's a funny xor that doesn't take a second argument ;-)

Anyway, let's not argue forever about the choice of this name.  It's
just the first thing that came to my mind from the original review, so I
used it as an indicator to see if you had done something about it.  It
seems I picked an indicator that is not significant for the overall
state.

-- 
Thomas Rast
t...@thomasrast.ch
--
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 issues with submodules

2013-11-22 Thread Jens Lehmann
Am 22.11.2013 17:12, schrieb Ramkumar Ramachandra:
 Jeff King wrote:
 I just checked it out: it uses `git ls-files -m` to get the list of
 unstaged changes; `git diff --name-only HEAD --` will list staged
 changes as well.

 That diff command compares the working tree and HEAD; if you are trying
 to match `ls-files -m`, you probably wanted just `git diff --name-only`
 to compare the working tree and the index. Although in a script you'd
 probably want to use the plumbing `git diff-files` instead.
 
 Thanks for that. It's probably not worth fixing ls-files; I'll patch
 Arcanist to use diff-files instead.

Good to have an short term solution for Sergey, but Heiko and I
discussed this issue and agreed that we should fix ls-files. After
all the user explicitly asked not to be bothered with submodule
differences by configuring the ignore setting.
--
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 issues with submodules

2013-11-22 Thread Jens Lehmann
Am 22.11.2013 19:11, schrieb Ramkumar Ramachandra:
 Sergey Sharybin wrote:
 To reproduce the issue:

 - Run git submodule update --recursive to make sure their SHA is
 changed. Then `git add /path/to/changed submodule` or just `git add .`
 - Modify any file from the parent repository
 - Neither of `git status`, `git diff` and `git diff-files --name-only`
 will show changes to a submodule, only changes to that file which was
 changed in parent repo.
 - Make a git commit. It will not list changes to submodule as wll.
 - `git show HEAD` will show changes to both file from and parent
 repository (which is expected) and will also show changes to the
 submodule hash (which is unexpected i'd say).
 
 Thanks Sergey; I can confirm that this is a bug.

Hmm, looks like git show also needs to be fixed to honor the
ignore setting from .gitmodules. It already does that for
diff.ignoreSubmodules from either .git/config or git -c and
also supports the --ignore-submodules command line option.
The following fixes this inconsistency for me:

--8---
diff --git a/builtin/log.c b/builtin/log.c
index b708517..ca97cfb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -25,6 +25,7 @@
 #include version.h
 #include mailmap.h
 #include gpg-interface.h
+#include submodule.h

 /* Set a default date-time format for git log (log.date config variable) */
 static const char *default_date_mode = NULL;
@@ -521,6 +522,7 @@ int cmd_show(int argc, const char **argv, const char *prefix
int i, count, ret = 0;

init_grep_defaults();
+   gitmodules_config();
git_config(git_log_config, NULL);

memset(match_all, 0, sizeof(match_all));
--8---

But the question is if that is the right thing to do: should
diff.ignoreSubmodules and submodule.name.ignore only affect
the diff family or also git log  friends? That would make
users blind for submodule history (which they already are
when using diff  friends, so that might be ok here too).

 For some reason, the
 `git add .` is adding the ignored submodule to the index.

The ignore setting is documented to only affect diff output
(including what checkout, commit and status show as modified).
While I agree that this behavior is confusing for Sergey and
not optimal for the floating branch model he uses, git is
currently doing exactly what it should. And for people using
the ignore setting to not having to stat submodules with huge
and/or many files that behavior is what they want: don't bother
me with what changed, but commit what I did change on purpose.
We may have to rethink what should happen for users of the
floating branch model though.

 After that,
 
   $ git diff-index @
 
 is not showing the ignored submodule.

Of course it isn't, it's configured not to. You'll have to use
--ignore-submodules=dirty to override the configuration to make
it show differences in the recorded hash.
--
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 issues with submodules

2013-11-22 Thread Sergey Sharybin
  For some reason, the
  `git add .` is adding the ignored submodule to the index.

 The ignore setting is documented to only affect diff output
 (including what checkout, commit and status show as modified).
 While I agree that this behavior is confusing for Sergey and
 not optimal for the floating branch model he uses, git is
 currently doing exactly what it should. And for people using
 the ignore setting to not having to stat submodules with huge
 and/or many files that behavior is what they want: don't bother
 me with what changed, but commit what I did change on purpose.
 We may have to rethink what should happen for users of the
 floating branch model though.


I totally see what's happening here and indeed current logic of `git
add .` agree is correct from how it was designed to. I could also see
why it might be useful to keep `git add .` and `git commit .` not to
respect submodule ignore flag. The only confusing thing here is that
if i stage changed submodule with this command i wouldn't see this
submodule in changes to be committed wen doing a commit.

So seems it's just matter of better communication of what's gonna to
be committed in changes to be committed section? Or maybe even make
it so `git status` will show staged changes from submdules hash
regardless ignore flag? Just an ideas how to make communication what's
going on a bit better :)

And for sure don't think suppressing stuff from git show is a nice
idea (if i understand your proposal f making submodule ignore option
affect on other commands).

-- 
With best regards, Sergey Sharybin
--
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: Git issues with submodules

2013-11-22 Thread Heiko Voigt
Hi,

On Fri, Nov 22, 2013 at 10:01:44PM +0100, Jens Lehmann wrote:
 Hmm, looks like git show also needs to be fixed to honor the
 ignore setting from .gitmodules. It already does that for
 diff.ignoreSubmodules from either .git/config or git -c and
 also supports the --ignore-submodules command line option.
 The following fixes this inconsistency for me:
 
 --8---
 diff --git a/builtin/log.c b/builtin/log.c
 index b708517..ca97cfb 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -25,6 +25,7 @@
  #include version.h
  #include mailmap.h
  #include gpg-interface.h
 +#include submodule.h
 
  /* Set a default date-time format for git log (log.date config variable) */
  static const char *default_date_mode = NULL;
 @@ -521,6 +522,7 @@ int cmd_show(int argc, const char **argv, const char 
 *prefix
 int i, count, ret = 0;
 
 init_grep_defaults();
 +   gitmodules_config();
 git_config(git_log_config, NULL);
 
 memset(match_all, 0, sizeof(match_all));
 --8---
 
 But the question is if that is the right thing to do: should
 diff.ignoreSubmodules and submodule.name.ignore only affect
 the diff family or also git log  friends? That would make
 users blind for submodule history (which they already are
 when using diff  friends, so that might be ok here too).
 
  For some reason, the
  `git add .` is adding the ignored submodule to the index.
 
 The ignore setting is documented to only affect diff output
 (including what checkout, commit and status show as modified).
 While I agree that this behavior is confusing for Sergey and
 not optimal for the floating branch model he uses, git is
 currently doing exactly what it should. And for people using
 the ignore setting to not having to stat submodules with huge
 and/or many files that behavior is what they want: don't bother
 me with what changed, but commit what I did change on purpose.
 We may have to rethink what should happen for users of the
 floating branch model though.

This gets more nasty. When using 'git add .' you secretly add the
submodule to the index. But it is neither shown in status nor diff
--cached. commit actually complains there is nothing to add. But then
once you add a local file to the index you can commit and secretly take
the submodule change with you.

What I think needs fixing here first is that the ignore setting should not
apply to any diffs between HEAD and index. IMO, it should only apply
to the diff between worktree and index.

When we have that the user does not see the submodule changed when
normally working. But after doing git add . the change to the submodule
should be shown in status and diff regardless of the configuration.

I will have a look at that.

After that we can discuss whether add should add submodules that are
tracked but not shown. How about commit -a ? Should it also ignore the
change? I am undecided here. There does not seem to be any good
decision. From the users point of view we should probably not add it
since its not visible in status. What do others think?

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: Git issues with submodules

2013-11-22 Thread Jonathan Nieder
Heiko Voigt wrote:

 After that we can discuss whether add should add submodules that are
 tracked but not shown. How about commit -a ? Should it also ignore the
 change? I am undecided here. There does not seem to be any good
 decision. From the users point of view we should probably not add it
 since its not visible in status. What do others think?

I agree --- it should not add.

That leaves the question of how to add explicitly.  git add -f?
git add --ignore-submodules=none?

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: What's cooking in git.git (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Jeff King
On Fri, Nov 22, 2013 at 12:05:25PM -0800, Junio C Hamano wrote:

  If this could make it to `next` some time next week, that would work
  out great for us, because we may start considering using `next` as a
  partial or full deployment on our production machines
 
 I do not think potentially incompatible stuff that are slated for
 2.0 that have been cooking in 'next' affects the server side, so
 that may be a good and safe move.

I do not think we will literally run `next` in this case, but probably
v1.8.5 + selected topics (like this one :) ).

We do not need to base ourselves on a release, of course, and we may
start using a rolling version of master, but choose quiescent points in
the cycle (like starting with a release, and then rolling forward around
-rc time). I started trying that with this cycle, which is how I found
the --literal-pathspec regression in mid-cycle, and then found out the
fix hadn't graduated during -rc. :)

If that proves stable, then I will consider bumping up our frequency of
following `master`, and then eventually following `next` (possibly with
some lag). As a large site, we get to expose the code to a lot of new
people; but we also need to be mindful that we are exposing a lot of
people to new bugs.

-Peff
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Jeff King
On Fri, Nov 22, 2013 at 06:58:55PM +0100, Thomas Rast wrote:

  I didn't touch that. Vicent, did you have a comment on the name (it
  really does look like it is a negation, and the only caller is
  ewah_not).
 
 Hmm, so it really was that one unlucky thing :-)

I don't promise there is only one unlucky thing. :) Only that we made a
good faith effort to address the comments. There were a lot of comments
and a lot of re-rolls, and I would not be surprised if something else
was missed (I am not thinking of anything in particular, but just
preparing you mentally).

 I don't have much to say on the area, but if you think it helps you I
 can set aside some time RSN to review the second half of the series,
 too.  Back in June I only looked at the first half.

I would love that. My comments to Junio were not to rush the topic, but
mainly to keep it progressing.

Re-rolling such a big chunk of code _is_ a pain for both me and for
reviewers, so I wouldn't mind switching to fixes on top instead of
re-rolling at some point. But we can do another round or two of re-roll
first.

-Peff
--
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 (Nov 2013, #05; Thu, 21)

2013-11-22 Thread Thomas Rast
Jeff King p...@peff.net writes:

 Re-rolling such a big chunk of code _is_ a pain for both me and for
 reviewers, so I wouldn't mind switching to fixes on top instead of
 re-rolling at some point. But we can do another round or two of re-roll
 first.

No, actually I think the plan that you sketched in the other side thread
sounded nice: give it some exposure in next.  I'll still try and read
the rest, but that way it hopefully gets (much) more testing.

-- 
Thomas Rast
t...@thomasrast.ch
--
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] git p4: Use git diff-tree instead of format-patch

2013-11-22 Thread Pete Wyckoff
gits...@pobox.com wrote on Thu, 21 Nov 2013 11:47 -0800:
 Crestez Dan Leonard cdleon...@gmail.com writes:
 
  The output of git format-patch can vary with user preferences. In
  particular setting diff.noprefix will break the git apply that
  is done as part of git p4 submit.
 
  Signed-off-by: Crestez Dan Leonard cdleon...@gmail.com
  ---
   git-p4.py | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/git-p4.py b/git-p4.py
  index 31e71ff..fe988ce 100755
  --- a/git-p4.py
  +++ b/git-p4.py
  @@ -1308,7 +1308,7 @@ class P4Submit(Command, P4UserMap):
   else:
   die(unknown modifier %s for %s % (modifier, path))
   
  -diffcmd = git format-patch -k --stdout \%s^\..\%s\ % (id, id)
  +diffcmd = git diff-tree -p \%s\ % (id)
   patchcmd = diffcmd +  | git apply 
   tryPatchCmd = patchcmd + --check -
   applyPatchCmd = patchcmd + --check --apply -
 
 I do not do p4 myself, but from a cursory reading it looks like the
 right thing to do.  Thanks.
 
 The output of git shortlog --no-merges --since=1.year git-p4.py
 tells me that Pete should be the person much more familiar with the
 code than myself, so I'll Cc him just in case...

This looks great, and passes all my tests.

Acked-by: Pete Wyckoff p...@padd.com

Thanks,

-- Pete
--
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] git p4: Use git diff-tree instead of format-patch

2013-11-22 Thread Junio C Hamano
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


Re: [PATCH] drop support for experimental loose objects

2013-11-22 Thread Jonathan Nieder
Jeff King wrote:

  sha1_file.c|  74 
 -

Yay!

  t/t1013-loose-object-format.sh |  66 --

Hmm, not all of these tests are about the experimental format.  Do
we really want to remove them all?

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: [PATCH] drop support for experimental loose objects

2013-11-22 Thread Jeff King
On Fri, Nov 22, 2013 at 04:24:05PM -0800, Jonathan Nieder wrote:

   t/t1013-loose-object-format.sh |  66 --
 
 Hmm, not all of these tests are about the experimental format.  Do
 we really want to remove them all?

I think so. They were not all testing the experimental format, but they
were about making sure the is-it-experimental heuristic triggered
properly with various zlib settings.

Now that we do not apply that heuristic, there is nothing (in git) to
test. We feed the contents straight to zlib. We could keep the objects
with small window size as a test, but we are not really testing git; we
are testing zlib at that point.

-Peff
--
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] drop support for experimental loose objects

2013-11-22 Thread Jonathan Nieder
Jeff King wrote:
 On Fri, Nov 22, 2013 at 04:24:05PM -0800, Jonathan Nieder wrote:

  t/t1013-loose-object-format.sh |  66 --

 Hmm, not all of these tests are about the experimental format.  Do
 we really want to remove them all?

 I think so. They were not all testing the experimental format, but they
 were about making sure the is-it-experimental heuristic triggered
 properly with various zlib settings.

 Now that we do not apply that heuristic, there is nothing (in git) to
 test. We feed the contents straight to zlib.

Ok, makes sense.

In principle the tests are still useful as futureproofing in case git
starts to sanity-check the objects as a way to notice corruption
earlier or something.  But in practice, that kind of futureproofing is
probably not worth the extra tests to maintain.

For what it's worth,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
--
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


[RFC PATCH] disable complete ignorance of submodules for index - HEAD diff

2013-11-22 Thread Heiko Voigt
If the value of ignore for submodules is set to all we would not show
whats actually committed during status or diff. This can result in the
user committing unexpected submodule references. Lets be nicer and always
show whats in the index.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
This probably needs splitting up into two patches one for the
refactoring and one for the actual fix. It is also missing tests, but I
would first like to know what you think about this approach.

 builtin/diff.c | 43 +++
 diff.h |  2 +-
 submodule.c|  6 --
 wt-status.c|  3 +++
 4 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..e9a356c 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -249,6 +249,21 @@ static int builtin_diff_files(struct rev_info *revs, int 
argc, const char **argv
return run_diff_files(revs, options);
 }
 
+static int have_cached_option(int argc, const char **argv)
+{
+   int i;
+   for (i = 1; i  argc; i++) {
+   const char *arg = argv[i];
+   if (!strcmp(arg, --))
+   return 0;
+   else if (!strcmp(arg, --cached) ||
+!strcmp(arg, --staged)) {
+   return 1;
+   }
+   }
+   return 0;
+}
+
 int cmd_diff(int argc, const char **argv, const char *prefix)
 {
int i;
@@ -259,6 +274,7 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
struct blobinfo blob[2];
int nongit;
int result = 0;
+   int have_cached;
 
/*
 * We could get N tree-ish in the rev.pending_objects list.
@@ -305,6 +321,11 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
 
if (nongit)
die(_(Not a git repository));
+
+   have_cached = have_cached_option(argc, argv);
+   if (have_cached)
+   DIFF_OPT_SET(rev.diffopt, NO_IGNORE_SUBMODULE);
+
argc = setup_revisions(argc, argv, rev, NULL);
if (!rev.diffopt.output_format) {
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
@@ -319,22 +340,12 @@ int cmd_diff(int argc, const char **argv, const char 
*prefix)
 * Do we have --cached and not have a pending object, then
 * default to HEAD by hand.  Eek.
 */
-   if (!rev.pending.nr) {
-   int i;
-   for (i = 1; i  argc; i++) {
-   const char *arg = argv[i];
-   if (!strcmp(arg, --))
-   break;
-   else if (!strcmp(arg, --cached) ||
-!strcmp(arg, --staged)) {
-   add_head_to_pending(rev);
-   if (!rev.pending.nr) {
-   struct tree *tree;
-   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
-   add_pending_object(rev, tree-object, 
HEAD);
-   }
-   break;
-   }
+   if (!rev.pending.nr  have_cached) {
+   add_head_to_pending(rev);
+   if (!rev.pending.nr) {
+   struct tree *tree;
+   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+   add_pending_object(rev, tree-object, HEAD);
}
}
 
diff --git a/diff.h b/diff.h
index e342325..81561b3 100644
--- a/diff.h
+++ b/diff.h
@@ -64,7 +64,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_FIND_COPIES_HARDER  (1   6)
 #define DIFF_OPT_FOLLOW_RENAMES  (1   7)
 #define DIFF_OPT_RENAME_EMPTY(1   8)
-/* (1   9) unused */
+#define DIFF_OPT_NO_IGNORE_SUBMODULE (1   9)
 #define DIFF_OPT_HAS_CHANGES (1  10)
 #define DIFF_OPT_QUICK   (1  11)
 #define DIFF_OPT_NO_INDEX(1  12)
diff --git a/submodule.c b/submodule.c
index 1905d75..9d81712 100644
--- a/submodule.c
+++ b/submodule.c
@@ -301,9 +301,11 @@ void handle_ignore_submodules_arg(struct diff_options 
*diffopt,
DIFF_OPT_CLR(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
DIFF_OPT_CLR(diffopt, IGNORE_DIRTY_SUBMODULES);
 
-   if (!strcmp(arg, all))
+   if (!strcmp(arg, all)) {
+   if (DIFF_OPT_TST(diffopt, NO_IGNORE_SUBMODULE))
+   return;
DIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);
-   else if (!strcmp(arg, untracked))
+   } else if (!strcmp(arg, untracked))
DIFF_OPT_SET(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES);
else if (!strcmp(arg, dirty))
DIFF_OPT_SET(diffopt, IGNORE_DIRTY_SUBMODULES);
diff --git a/wt-status.c b/wt-status.c
index b4e44ba..34be1cc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -462,6 +462,9 @@ static void wt_status_collect_changes_index(struct 

gettext CTYPE for libc

2013-11-22 Thread Trần Ngọc Quân
Hello,

$ mkdir xyz
$ cd xyz
$ rmdir ../xyz
$ git status
fatal: Unable to read current working directory: Kh?ng c? t?p tin ho?c
th? m?c nh? v?y

So, somthing wrong with our charset.

$ strace git status 21 | grep open
open(/etc/ld.so.cache, O_RDONLY|O_CLOEXEC) = 3
open(/lib/i386-linux-gnu/libz.so.1, O_RDONLY|O_CLOEXEC) = 3
open(/lib/i386-linux-gnu/libcrypto.so.1.0.0, O_RDONLY|O_CLOEXEC) = 3
open(/lib/i386-linux-gnu/libpthread.so.0, O_RDONLY|O_CLOEXEC) = 3
open(/lib/i386-linux-gnu/libc.so.6, O_RDONLY|O_CLOEXEC) = 3
open(/lib/i386-linux-gnu/libdl.so.2, O_RDONLY|O_CLOEXEC) = 3
open(/dev/null, O_RDWR|O_LARGEFILE)   = 3
open(/usr/lib/locale/locale-archive, O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3
open(/usr/share/locale/locale.alias, O_RDONLY|O_CLOEXEC) = 3
open(/usr/share/locale/vi_VN/LC_MESSAGES/libc.mo, O_RDONLY) = -1
ENOENT (No such file or directory)
open(/usr/share/locale/vi/LC_MESSAGES/libc.mo, O_RDONLY) = -1 ENOENT
(No such file or directory)
open(/usr/share/locale-langpack/vi_VN/LC_MESSAGES/libc.mo, O_RDONLY) =
-1 ENOENT (No such file or directory)
open(/usr/share/locale-langpack/vi/LC_MESSAGES/libc.mo, O_RDONLY) = 3
open(/usr/lib/i386-linux-gnu/gconv/gconv-modules.cache, O_RDONLY) = 3

We will see, this string come from libc.mo
$ gettext --domain=libc No such file or directory
Không có tập tin hoặc thư mục như vậy

in git's gettext.c, it not allow CTYPE= for all domain, so we will set
this one individually. In this ex. I set it for libc:

$ git diff
diff --git a/gettext.c b/gettext.c
index 71e9545..abd3978 100644
--- a/gettext.c
+++ b/gettext.c
@@ -115,6 +115,7 @@ static void init_gettext_charset(const char *domain)
setlocale(LC_CTYPE, );
charset = locale_charset();
bind_textdomain_codeset(domain, charset);
+   bind_textdomain_codeset(libc, charset);
setlocale(LC_CTYPE, C);
 }

And it work as I expect!

-- 
Trần Ngọc Quân.

--
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 issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Jens Lehmann wrote:
 But the question is if that is the right thing to do: should
 diff.ignoreSubmodules and submodule.name.ignore only affect
 the diff family or also git log  friends? That would make
 users blind for submodule history (which they already are
 when using diff  friends, so that might be ok here too).

No, I think it's the wrong thing to do. We don't want to show false history.

 The ignore setting is documented to only affect diff output
 (including what checkout, commit and status show as modified).
 While I agree that this behavior is confusing for Sergey and
 not optimal for the floating branch model he uses, git is
 currently doing exactly what it should. And for people using
 the ignore setting to not having to stat submodules with huge
 and/or many files that behavior is what they want: don't bother
 me with what changed, but commit what I did change on purpose.
 We may have to rethink what should happen for users of the
 floating branch model though.

I'd argue that the only reason the diff-family is blind is because the
commit hash changes in the first place; if the hash didn't change (ie.
floating submodules were represented by 0s hash or something), we
wouldn't have this problem. The correct solution is to also make `git
add' blind.
--
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: Git issues with submodules

2013-11-22 Thread Ramkumar Ramachandra
Heiko Voigt wrote:
 What I think needs fixing here first is that the ignore setting should not
 apply to any diffs between HEAD and index. IMO, it should only apply
 to the diff between worktree and index.

 When we have that the user does not see the submodule changed when
 normally working. But after doing git add . the change to the submodule
 should be shown in status and diff regardless of the configuration.

Yeah, I think this is a good direction.

 After that we can discuss whether add should add submodules that are
 tracked but not shown. How about commit -a ? Should it also ignore the
 change?

Here, I think ignored submodules should behave like files matched by
.gitignore: add should not add (`add -f` would be a good way to force
it), and `commit -a` should also exclude it.
--
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