Re: [PATCH] drop support for "experimental" loose objects
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
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)
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
On Fri, Nov 22, 2013 at 10:58 AM, Jeff King 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
[+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
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
Hey, Answers are inlined. On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra 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
BTW, a workaround is to pass a path to askpass script in the GIT_ASKPASS environment variable. Jens Lehmann writes: > Am 16.11.2013 22:42, schrieb Thomas Rast: >> Dmitry Neverov 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
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" 2>&1 1>stdout) | 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" 2>actual &&
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
Sergey Sharybin wrote: > On Fri, Nov 22, 2013 at 5:16 PM, Ramkumar Ramachandra > 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
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
---
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/h
git submodule update needs to be at the toplevel of working tree, why?
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?
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
On Fri, Nov 22, 2013 at 12:24 PM, Jeff King 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?
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
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
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 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
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
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)
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 --- 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 -is compared against each commit between the fork-point and . -The diffs are compared after removing any whitespace and line numbers. +Determine whether there are commits in `..` that are +equivalent to those in the range `..`. -Every commit that doesn't exist in the branch -has its id (sha1) reported, prefixed by a symbol. The ones that have -equivalent change already -in the branch are prefixed with a minus (-) sign, and those -that only exist in the branch are prefixed with a plus (+) symbol: - - __*__*__*__*__> - / -fork-point - \__+__+__-__+__+__-__+__> - - -If a has been given then the commits along the branch up -to and including are not reported: - - __*__*__*__*__> - / -fork-point - \__*__*-__+__> - - -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 under a different commit id. For example, -this will happen if you're feeding patches 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 `..`, prefixed with +`-` for commits that have an equivalent in , and `+` for +commits that do not. OPTIONS --- -v:: - Verbose. + Show the commit subjects next to the SHA1s. :: - 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. :: Working branch; defaults to HEAD. @@ -59,6 +39,96 @@ OPTIONS :: 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 determine if a series of patches has been +applied by the upstream maintainer. In such a workflow you might +create and send a t
Re: Git issues with submodules
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)
Jeff King 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
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 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
Jeff King 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)
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
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
Krzesimir Nowak 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
> ---
>
> 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);
>
Re: Git issues with submodules
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 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 > 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)
Jeff King 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
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)
Thomas Rast writes: > NAME > > +git-cherry - Find commits not applied in upstream > > +Determine whether there are commits in `..` that are > +equivalent to those in the range `..`. > > +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 `..`, prefixed with > +`-` for commits that have an equivalent in , 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)
Junio C Hamano 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)
Jeff King 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)
Thomas Rast writes: > Junio C Hamano 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)
On Fri, Nov 22, 2013 at 6:26 PM, Jeff King 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)
On Fri, Nov 22, 2013 at 8:36 PM, Junio C Hamano 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)
Vicent Martí writes: > On Fri, Nov 22, 2013 at 8:36 PM, Junio C Hamano 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)
Vicent Marti writes: > On Fri, Nov 22, 2013 at 6:26 PM, Jeff King 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
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
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..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
> > 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
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..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
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)
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)
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)
Jeff King 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
gits...@pobox.com wrote on Thu, 21 Nov 2013 11:47 -0800:
> Crestez Dan Leonard 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
> > ---
> > 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
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
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
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
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
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 -- 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
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
---
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 w
gettext CTYPE for libc
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 2>&1 | 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
Jens Lehmann wrote: > But the question is if that is the right thing to do: should > diff.ignoreSubmodules and submodule..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
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
