Git development
 help / color / mirror / Atom feed
* Re: How to delete large files
From: Mikael Magnusson @ 2009-10-02 12:26 UTC (permalink / raw)
  To: Nils Homer; +Cc: git
In-Reply-To: <C6EB1E10.D7AB%nilshomer@gmail.com>

2009/10/2 Nils Homer <nilshomer@gmail.com>:
> I wish to delete some large offending files (10MB to >100MB each) from my
> git repository (git://bfast.git.sourceforge.net/gitroot/bfast/bfast).  The
> current size of the repo is 656MB.
>
> I created a backup of my repository and then searched for such offending
> files using a script found here:
> http://stubbisms.wordpress.com/2009/07/10/git-script-to-show-largest-pack-ob
> jects-and-trim-your-waist-line/
> where I modified the script to output in MB instead of KB.
>
> This gave me a list of files that I wanted to delete:
> -e   size  pack                                      SHA
> location
> 113  113   f9f2faab597d4f8ccbfac2864347dbc256353fbf
> tests.long/save/save.tar.gz
> 113  113   926b1ba880a26354c4a6b9391985f57fbc9a1174
> tests.long/save/save.tar.gz
> 113  113   e568480bcb8239e6d1ed8d2da86c309c0d3d101b
> tests.long/save/save.tar.gz
> 113  113   e3c0ee53f20e8ebfb60eaefcd7b405168c26a565
> tests.long/save/save.tar.gz
> 103  103   ee2ee50c5075d05d29764c8d4b9acc2acedda919
> tests.long/save/save.tar.gz
> 35   35    319c75945c27096093dbab5a0bf6a9a08089bc2d
> tests.long/data/data.tar.gz
> 11   11    805193c74ceeffca9da3a2788545e701d77e1caf  tests/save/save.tar.gz
> 11   11    658e4a78c1028875ab597d6bde5823cd6a1694b9  tests/save/save.tar.gz
>
> So I decided to remove these files using:
> git filter-branch --index-filter "git rm -rf --cached --ignore-unmatch
> tests.long/save/save.tar.gz tests.long/data/data.tar.gz
> tests/save/save.tar.gz" HEAD
>
> I then ran:
> rm -rf .git/refs/original
> git reflog expire --expire=now 卟ll
> git gc --prune=now
>
> Still (using du 虐) the repository is 656MB and I can see the above files in
> the revision list:
> git rev-list --all --objects | grep tests.long/save/save.tar.gz
> ee2ee50c5075d05d29764c8d4b9acc2acedda919 tests.long/save/save.tar.gz
> e568480bcb8239e6d1ed8d2da86c309c0d3d101b tests.long/save/save.tar.gz
> f9f2faab597d4f8ccbfac2864347dbc256353fbf tests.long/save/save.tar.gz
> 926b1ba880a26354c4a6b9391985f57fbc9a1174 tests.long/save/save.tar.gz
> e3c0ee53f20e8ebfb60eaefcd7b405168c26a565 tests.long/save/save.tar.gz
>
> Could this be because of tags that I had previously created?
>
> I am running git version 1.6.3.3.  I appreciate any help in advance,

Well, you just gave "HEAD" to git filter-branch to rewrite, i think
you want --all to rewrite all refs you have.

-- 
Mikael Magnusson

^ permalink raw reply

* Re: How to delete large files
From: Matthieu Moy @ 2009-10-02 11:41 UTC (permalink / raw)
  To: Nils Homer; +Cc: git
In-Reply-To: <C6EB1E10.D7AB%nilshomer@gmail.com>

Nils Homer <nilshomer@gmail.com> writes:

> Still (using du ­h) the repository is 656MB and I can see the above files in
> the revision list:
> git rev-list --all --objects | grep tests.long/save/save.tar.gz
> ee2ee50c5075d05d29764c8d4b9acc2acedda919 tests.long/save/save.tar.gz
> e568480bcb8239e6d1ed8d2da86c309c0d3d101b tests.long/save/save.tar.gz
> f9f2faab597d4f8ccbfac2864347dbc256353fbf tests.long/save/save.tar.gz
> 926b1ba880a26354c4a6b9391985f57fbc9a1174 tests.long/save/save.tar.gz
> e3c0ee53f20e8ebfb60eaefcd7b405168c26a565 tests.long/save/save.tar.gz
>
> Could this be because of tags that I had previously created?

I guess so. Anything pointing to the old history will prevent Git from
deleting it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* How to delete large files
From: Nils Homer @ 2009-10-02 10:03 UTC (permalink / raw)
  To: git

I wish to delete some large offending files (10MB to >100MB each) from my
git repository (git://bfast.git.sourceforge.net/gitroot/bfast/bfast).  The
current size of the repo is 656MB.

I created a backup of my repository and then searched for such offending
files using a script found here:
http://stubbisms.wordpress.com/2009/07/10/git-script-to-show-largest-pack-ob
jects-and-trim-your-waist-line/
where I modified the script to output in MB instead of KB.

This gave me a list of files that I wanted to delete:
-e   size  pack                                      SHA
location
113  113   f9f2faab597d4f8ccbfac2864347dbc256353fbf
tests.long/save/save.tar.gz
113  113   926b1ba880a26354c4a6b9391985f57fbc9a1174
tests.long/save/save.tar.gz
113  113   e568480bcb8239e6d1ed8d2da86c309c0d3d101b
tests.long/save/save.tar.gz
113  113   e3c0ee53f20e8ebfb60eaefcd7b405168c26a565
tests.long/save/save.tar.gz
103  103   ee2ee50c5075d05d29764c8d4b9acc2acedda919
tests.long/save/save.tar.gz
35   35    319c75945c27096093dbab5a0bf6a9a08089bc2d
tests.long/data/data.tar.gz
11   11    805193c74ceeffca9da3a2788545e701d77e1caf  tests/save/save.tar.gz
11   11    658e4a78c1028875ab597d6bde5823cd6a1694b9  tests/save/save.tar.gz

So I decided to remove these files using:
git filter-branch --index-filter "git rm -rf --cached --ignore-unmatch
tests.long/save/save.tar.gz tests.long/data/data.tar.gz
tests/save/save.tar.gz" HEAD

I then ran:
rm -rf .git/refs/original
git reflog expire --expire=now ‹all
git gc --prune=now

Still (using du ­h) the repository is 656MB and I can see the above files in
the revision list:
git rev-list --all --objects | grep tests.long/save/save.tar.gz
ee2ee50c5075d05d29764c8d4b9acc2acedda919 tests.long/save/save.tar.gz
e568480bcb8239e6d1ed8d2da86c309c0d3d101b tests.long/save/save.tar.gz
f9f2faab597d4f8ccbfac2864347dbc256353fbf tests.long/save/save.tar.gz
926b1ba880a26354c4a6b9391985f57fbc9a1174 tests.long/save/save.tar.gz
e3c0ee53f20e8ebfb60eaefcd7b405168c26a565 tests.long/save/save.tar.gz

Could this be because of tags that I had previously created?

I am running git version 1.6.3.3.  I appreciate any help in advance,

Nils Homer

^ permalink raw reply

* Re: [PATCH 1/2] do not mangle short options which take arguments
From: Johannes Schindelin @ 2009-10-02  9:04 UTC (permalink / raw)
  To: Jeff King; +Cc: Clemens Buchacher, Shawn O. Pearce, git
In-Reply-To: <20091002084359.GA8878@coredump.intra.peff.net>

Hi,

On Fri, 2 Oct 2009, Jeff King wrote:

> On Fri, Oct 02, 2009 at 10:42:36AM +0200, Johannes Schindelin wrote:
> 
> > > Yes, we deliberately allow users to shoot themselves in the foot. But 
> > > they should have to use at least a long option to do it.
> > 
> > Something like this?
> > [...]
> > +		} else if (!strcmp(cmd, "--shoot-me-in-the-foot")) {
> > +			warning ("Bang, bang!");
> 
> Doh! Now I have to come up with a new joke patch for the GitTogether!

Don't make me envious :-(

Ciao,
Dscho

^ permalink raw reply

* Re: MSVC build broken (on cygwin)
From: Marius Storm-Olsen @ 2009-10-02  8:49 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Ramsay Jones, GIT Mailing-list, Shawn O. Pearce
In-Reply-To: <81b0412b0910020123j13c74497w874e301c38cddec9@mail.gmail.com>

Alex Riesen said the following on 02.10.2009 10:23:
> MSVC (all versions) define a compiler specific _MSC_VER, if that's of any use.

In this case it was define guards to let both MSVC and MinGW through 
:) Both use _WIN32 and WIN32, which Cygwin gcc normally doesn't, 
unless, as Ramsay said, you specify -mno-cygwin, or include windows.h 
apparently.

Maybe we should allow Cygwin to also include the LEAN_AND_MEAN 
windows.h in git-compat-util.h, and rather fix up the guards to 
cleanly differ between Cygwin and non-Cygwin on Windows?

Apparently, nothing is broken in neither Cygwin, MinGW or MSVC after 
Ramsays whitespace fix, but I'm sure it might get hairy later, if/when 
we get more Windows contributions. Keeping the guards right could get 
tricky.

So, something like this maybe, in git-compat-util.h:

#if defined(__MINGW32__) || defined(_MSC_VER)
#  defined API_WIN32
#  defined OS_WINDOWS
#elif defined(__CYGWIN__)
#  defined API_POSIX
#  defined OS_WINDOWS
#else
#  defined API_POSIX
#endif

So, then we can use #ifdef API_WIN32 when using the Win32 API is the 
only option/preferred for MinGW or MSVC; and use #ifdef OS_WINDOWS 
when there are things that affect all the Windows builds.

Opinions?

--
.marius

^ permalink raw reply

* Re: [PATCH 1/2] do not mangle short options which take arguments
From: Jeff King @ 2009-10-02  8:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Clemens Buchacher, Shawn O. Pearce, git
In-Reply-To: <alpine.DEB.1.00.0910021042110.18640@intel-tinevez-2-302>

On Fri, Oct 02, 2009 at 10:42:36AM +0200, Johannes Schindelin wrote:

> > Yes, we deliberately allow users to shoot themselves in the foot. But 
> > they should have to use at least a long option to do it.
> 
> Something like this?
> [...]
> +		} else if (!strcmp(cmd, "--shoot-me-in-the-foot")) {
> +			warning ("Bang, bang!");

Doh! Now I have to come up with a new joke patch for the GitTogether!

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] do not mangle short options which take arguments
From: Johannes Schindelin @ 2009-10-02  8:42 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Jeff King, Shawn O. Pearce, git
In-Reply-To: <20091002073628.GA9444@localhost>

Hi,

On Fri, 2 Oct 2009, Clemens Buchacher wrote:

> On Fri, Oct 02, 2009 at 02:11:59AM -0400, Jeff King wrote:
> 
> > So I don't feel _too_ strongly. I am just concerned that we are 
> > introducing some DWYM magic that is not really going to help many 
> > people out, and is just going to make understanding the rules for 
> > option parsing more complex, and introduce the possibility (albeit 
> > slim) of breaking people's scripts and trained fingers.
> 
> But it makes the rules simpler, because it removes ambiguities such as 
> the one above.
> 
> Yes, we deliberately allow users to shoot themselves in the foot. But 
> they should have to use at least a long option to do it.

Something like this?

-- snipsnap --
[PATCH] Deliberately allow users to shoot themselves in the foot

But they should have to use at least a long option to do it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git.c b/git.c
index 9883009..e7cb946 100644
--- a/git.c
+++ b/git.c
@@ -122,6 +122,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--shoot-me-in-the-foot")) {
+			warning ("Bang, bang!");
 		} else {
 			fprintf(stderr, "Unknown option: %s\n", cmd);
 			usage(git_usage_string);
-- 
1.6.4.297.gcb4cc

^ permalink raw reply related

* Re: [PATCH 2/2] allow mangling short options which take integer arguments
From: Johannes Schindelin @ 2009-10-02  8:41 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Jeff King, Shawn O. Pearce, git
In-Reply-To: <20091002082633.GA21168@localhost>

Hi,

On Fri, 2 Oct 2009, Clemens Buchacher wrote:

> On Fri, Oct 02, 2009 at 03:50:12AM -0400, Jeff King wrote:
> > On Fri, Oct 02, 2009 at 09:43:17AM +0200, Clemens Buchacher wrote:
> > 
> > > On Thu, Oct 01, 2009 at 11:55:03PM +0200, Johannes Schindelin wrote:
> > > 
> > > > And this patch looks even more straight-forward than 1/2, _but_... what 
> > > > about cases where there are short options that are digits?
> > > 
> > > Could you point me to one of those? I did not find any during my
> > > non-exhaustive search. We should be able to handle them easily by adding
> > > PARSE_OPT_MANY.
> > 
> > The one that comes readily to mind is "git log -1", but that is actually
> > parsed by the revision options parser, which doesn't use parseopt. But
> > there are a few done by parseopt:
> 
> Oh, I mistakenly thought Dscho was asking about options with 
> single-digit arguments. Thanks for clearing this up.

I was actually thinking both of "git log -11" as well as out-of-tree users 
of parse-options.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] tests: make all test files executable
From: Jeff King @ 2009-10-02  8:39 UTC (permalink / raw)
  To: Mark Rada; +Cc: Junio C Hamano, git
In-Reply-To: <20091002080134.GD27664@coredump.intra.peff.net>

On Fri, Oct 02, 2009 at 04:01:34AM -0400, Jeff King wrote:

> >  0 files changed, 0 insertions(+), 0 deletions(-)
> >  mode change 100644 => 100755 t/t5531-deep-submodule-push.sh
> >  mode change 100644 => 100755 t/t9501-gitweb-standalone-http-status.sh
> > 
> > diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> > old mode 100644
> > new mode 100755
> > diff --git a/t/t9501-gitweb-standalone-http-status.sh
> > b/t/t9501-gitweb-standalone-http-status.sh
> > old mode 100644
> > new mode 100755
> 
> When applying via "am", I only got the first change in my tree. I'll see
> if I can confirm and make a test case.

Ah, nevermind. The problem is that your patch was word-wrapped, making
the second "diff --git" line bogus. It would have been nice to have it
print a warning instead of silently ignoring that bit of the patch.

I'm not sure if it is a good idea, though. git-apply sees that the "diff
--git" line is there but can't get a filename from it. Which _should_
cause it to barf when we get to the mode lines. But we first see the
wrapped filename and consider that patch section to be over (and the
mode lines just become cruft at the end of the patch, which we must
ignore).

And we must consider that patch section to be over when we see random
text, because we need to jump back to the outer parser looking for more
hunks. We could warn about a "diff --git" line that has no actual
changes associated with it, though that is certainly a user-visible
change. I also suspect it wouldn't help if there was a mode change
followed by some actual content changes.

Hmph.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] allow mangling short options which take integer arguments
From: Clemens Buchacher @ 2009-10-02  8:26 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, Shawn O. Pearce, git
In-Reply-To: <20091002075012.GB27664@coredump.intra.peff.net>

On Fri, Oct 02, 2009 at 03:50:12AM -0400, Jeff King wrote:
> On Fri, Oct 02, 2009 at 09:43:17AM +0200, Clemens Buchacher wrote:
> 
> > On Thu, Oct 01, 2009 at 11:55:03PM +0200, Johannes Schindelin wrote:
> > 
> > > And this patch looks even more straight-forward than 1/2, _but_... what 
> > > about cases where there are short options that are digits?
> > 
> > Could you point me to one of those? I did not find any during my
> > non-exhaustive search. We should be able to handle them easily by adding
> > PARSE_OPT_MANY.
> 
> The one that comes readily to mind is "git log -1", but that is actually
> parsed by the revision options parser, which doesn't use parseopt. But
> there are a few done by parseopt:

Oh, I mistakenly thought Dscho was asking about options with single-digit
arguments. Thanks for clearing this up.

Clemens

^ permalink raw reply

* Re: MSVC build broken (on cygwin)
From: Alex Riesen @ 2009-10-02  8:23 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Ramsay Jones, GIT Mailing-list, Shawn O. Pearce
In-Reply-To: <4AC5B4AE.5070307@gmail.com>

MSVC (all versions) define a compiler specific _MSC_VER, if that's of any use.

^ permalink raw reply

* Re: MSVC build broken (on cygwin)
From: Marius Storm-Olsen @ 2009-10-02  8:07 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list, Shawn O. Pearce
In-Reply-To: <4AC4E2C2.6030509@ramsay1.demon.co.uk>

Ramsay Jones said the following on 01.10.2009 19:11:
> Hi Marius,
> 
> I know that I'm somewhat late to comment on your recent MSVC
> build patches, but I was busy at the time; better late than
> never... maybe ;-)
> 
> While the patches were traversing the list, I was feeling
> somewhat nervous about the effect of the patches on the
> cygwin build; in fact I remember thinking that they had
> *probably* broken the build. But I was busy...
> 
> Well I finally found time, yesterday, to take a closer look.
> I spent 10-15 minutes squinting at the code in order to
> convince myself that you had in fact *not* broken the cygwin
> build. :)
...
> [Note: I was a little surprised that it got that far, since I didn't
> expect it to find the zlib header files. However, I have set the
> INCLUDE environment variable which msvc is respecting! yeah, a bit old
> fashioned!  Having also set the LIB environment variable, I was then
> a bit surprised that the linker didn't find the library; until I
> noticed that my library is called libz.lib *not* zlib.lib!]
...
Clone the git://repo.or.cz/msvcgit.git, and run the 
setup_32bit_env.cmd script in there, and you should have everything 
you need to both compile and link Git with MSVC.

> Note that the patch below includes some line-wrapping which you can
> ignore if you like, it just makes the Makefile easier to read.
> The only change that matters is inserting a space between -DWIN32 and
> -D_CONSOLE.
> 
> Anyway, the point is *not* to get the msvc build to work for me; rather
> it is to understand why the build *works* for you. ;-)

First of all, thanks for the thorough report! :)
Second, I just recompiled, and it magically works for me. Why is a 
good question, since I also think it shouldn't at this point. The 
_WIN32 define is added by the compiler, and the WIN32 is added by 
windows.h, so our define guards *should* be testing for the _WIN32 define.

To how the guarded code reacts, I preprocessed the run-command.c with 
both version of the command line, and the result was the same:

( 9:54:49 - D:\msvc\git)
 > cl -E ... -DWIN32-D_CONSOLE ... run-command.c | grep run_thread
run-command.c
static unsigned __stdcall run_thread(void *data)
         async->tid = (HANDLE) _beginthreadex(((void *)0), 0, 
run_thread, async, 0, ((void *)0));

( 9:55:43 - D:\msvc\git)
 > cl -E ... -DWIN32 -D_CONSOLE ... run-command.c | grep run_thread
run-command.c
static unsigned __stdcall run_thread(void *data)
         async->tid = (HANDLE) _beginthreadex(((void *)0), 0, 
run_thread, async, 0, ((void *)0));

So, obviously, some magic in there is making it work for me. I have a 
hard time locating the magic in question though. :-/
That being said, does adding the space between the defines fix the 
MSVC compilation using Cygwin's GNU Make? It's none-the-less a correct 
patch, so you get an ack from me. Thanks!

Acked-by: Marius Storm-Olsen <mstormo@gmail.com>

--
.marius

^ permalink raw reply

* Re: [PATCH] tests: make all test files executable
From: Jeff King @ 2009-10-02  8:01 UTC (permalink / raw)
  To: Mark Rada; +Cc: Junio C Hamano, git
In-Reply-To: <4AC55E78.7010109@mailservices.uwaterloo.ca>

On Thu, Oct 01, 2009 at 09:59:20PM -0400, Mark Rada wrote:

> For consistency with the rest of the test files.

Thanks. Interestingly, though, I think this may be triggering a bug in
git-am:

>  0 files changed, 0 insertions(+), 0 deletions(-)
>  mode change 100644 => 100755 t/t5531-deep-submodule-push.sh
>  mode change 100644 => 100755 t/t9501-gitweb-standalone-http-status.sh
> 
> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> old mode 100644
> new mode 100755
> diff --git a/t/t9501-gitweb-standalone-http-status.sh
> b/t/t9501-gitweb-standalone-http-status.sh
> old mode 100644
> new mode 100755

When applying via "am", I only got the first change in my tree. I'll see
if I can confirm and make a test case.

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] do not mangle short options which take arguments
From: Jeff King @ 2009-10-02  7:57 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Shawn O. Pearce, git
In-Reply-To: <20091002073628.GA9444@localhost>

On Fri, Oct 02, 2009 at 09:36:28AM +0200, Clemens Buchacher wrote:

> Yes, that syntax looks reasonable. I expect this to be more involved, so I
> will rework the patch once we agree on whether or not we want it at all.

Thanks.

> Yes, that can happen. On the other hand, the "-ammend" typo actually did
> happen.

It did, but we are only guessing at how many people will be disrupted by
the new rule. That being said...

> And what I'm even more worried about are ambiguities like
> 
>   $ git commit -uno <path>
>   $ git commit -nou <path>
> 
> which are interpreted as one of
> 
>   $ git commit --untracked-files=no <path>
>   $ git commit --untracked-files --no-verify --only <path>

Making this clearer is a much more compelling argument to me. Though I
thought it was customary (not just for git, but for other programs) that
a short option that takes a parameter (even an optional one) would
consume the rest of a short options string. Still, it is a potential
source of confusion.

> > On the other hand, the cuddled value already has some DWYM magic (it
> > recognizes -amend), so it is already a little bit unsafe to use
> 
> Well, an error message is a lot safer than executing something you did not
> intend.

It's also an error exit code, which can affect how a script performs
(e.g., "git diff --exit-code"). But I don't have any real examples off
the top of my head of how this could be particularly disastrous, so feel
free to dismiss that as pushing too far into the hypothetical.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] allow mangling short options which take integer arguments
From: Jeff King @ 2009-10-02  7:50 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Johannes Schindelin, Shawn O. Pearce, git
In-Reply-To: <20091002074317.GB9444@localhost>

On Fri, Oct 02, 2009 at 09:43:17AM +0200, Clemens Buchacher wrote:

> On Thu, Oct 01, 2009 at 11:55:03PM +0200, Johannes Schindelin wrote:
> 
> > And this patch looks even more straight-forward than 1/2, _but_... what 
> > about cases where there are short options that are digits?
> 
> Could you point me to one of those? I did not find any during my
> non-exhaustive search. We should be able to handle them easily by adding
> PARSE_OPT_MANY.

The one that comes readily to mind is "git log -1", but that is actually
parsed by the revision options parser, which doesn't use parseopt. But
there are a few done by parseopt:

  $ git grep "OPT_.*'[0-9]'"
  archive.c:              OPT__COMPR('1', &compression_level, "compress faster", 1),
  archive.c:              OPT__COMPR_HIDDEN('2', &compression_level, 2),
  archive.c:              OPT__COMPR_HIDDEN('3', &compression_level, 3),
  archive.c:              OPT__COMPR_HIDDEN('4', &compression_level, 4),
  archive.c:              OPT__COMPR_HIDDEN('5', &compression_level, 5),
  archive.c:              OPT__COMPR_HIDDEN('6', &compression_level, 6),
  archive.c:              OPT__COMPR_HIDDEN('7', &compression_level, 7),
  archive.c:              OPT__COMPR_HIDDEN('8', &compression_level, 8),
  archive.c:              OPT__COMPR('9', &compression_level, "compress better", 9),
  builtin-checkout.c:             OPT_SET_INT('2', "ours", &opts.writeout_stage, "stage",
  builtin-checkout.c:             OPT_SET_INT('3', "theirs", &opts.writeout_stage, "stage",

-Peff

^ permalink raw reply

* Re: [PATCH 1/2] do not mangle short options which take arguments
From: Paolo Bonzini @ 2009-10-02  7:46 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Jeff King, Shawn O. Pearce, git
In-Reply-To: <20091002073628.GA9444@localhost>

On 10/02/2009 09:36 AM, Clemens Buchacher wrote:
> Well, an error message is a lot safer than executing something you did not
> intend.

If this is just for double --amend typos I don't see the need.  What you 
intended is just one "git rebase -i HEAD^^" away.

Notice that in Shawn's original example the guy actually passed -a too, 
so he would not even have the problem of overwriting the index due to -a.

If you have bouncy fingers, you can just make an alias

    ammend-fix = git reset --soft HEAD^ && git gui citool

Paolo

^ permalink raw reply

* Re: [PATCH] filter-branch: add --prune-empty to option summary
From: Jeff King @ 2009-10-02  7:45 UTC (permalink / raw)
  To: Adam Brewster; +Cc: git
In-Reply-To: <1254444731-6852-1-git-send-email-adambrewster@gmail.com>

On Thu, Oct 01, 2009 at 08:52:11PM -0400, Adam Brewster wrote:

> diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
> index 451950b..2cc3bd8 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -12,6 +12,7 @@ SYNOPSIS
>  	[--index-filter <command>] [--parent-filter <command>]
>  	[--msg-filter <command>] [--commit-filter <command>]
>  	[--tag-name-filter <command>] [--subdirectory-filter <directory>]
> +	[--prune-empty]
>  	[--original <namespace>] [-d <directory>] [-f | --force]
>  	[--] [<rev-list options>...]

Thanks. This makes sense given the existing structure, though I have to
wonder how useful some of these gigantic synopses really are. Do we
really need to list all of the options here in a cluttered, annoyingly
wrapped format, when we are just going to list them in a nice
easy-to-read format with their descriptions later on in the page?

And this is really not so much to do with your patch as a meta-rant, so
feel free to stop reading.

What should the synopsis really be communicating? In something like
git-cat-file(1), the synopsis nicely shows a few key facts:

  1. There are two "modes" if invocation.

  2. In one mode, you give an action and an object on the command line.

  3. In the other mode, you give --batch and feed some stuff over stdin.

Those are all useful pieces of information, and they are communicated
more quickly than they would be by forcing me to read the
paragraph-formatted text of the description section.

But look at the 18-line synopsis for git-grep or the 17-line one for
git-format-patch. Are they really helping anyone? (And it is not just
the line count. The 18-line synopsis for git-config is actually useful,
because there really are 18 modes of operation).

Non-git programs seem to take a more sparse approach. For example, some
one-line synopses from my system:

       sed [OPTION]... {script-only-if-no-other-script} [input-file]...
       ls [OPTION]... [FILE]...

or even this longer one:

       vim [options] [file ..]
       vim [options] -
       vim [options] -t tag
       vim [options] -q [errorfile]

Those all communicate some useful information (how to generally invoke
the program) cluttering it with redundant information.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] allow mangling short options which take integer arguments
From: Clemens Buchacher @ 2009-10-02  7:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.DEB.1.00.0910012354080.4985@pacific.mpi-cbg.de>

On Thu, Oct 01, 2009 at 11:55:03PM +0200, Johannes Schindelin wrote:

> And this patch looks even more straight-forward than 1/2, _but_... what 
> about cases where there are short options that are digits?

Could you point me to one of those? I did not find any during my
non-exhaustive search. We should be able to handle them easily by adding
PARSE_OPT_MANY.

Clemens

^ permalink raw reply

* Re: [PATCH 1/2] do not mangle short options which take arguments
From: Clemens Buchacher @ 2009-10-02  7:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20091002061159.GA24892@coredump.intra.peff.net>

On Fri, Oct 02, 2009 at 02:11:59AM -0400, Jeff King wrote:

> I thought the proposal was to disallow just cuddling of the value when
> the switch was combined with others. So you would disallow "git commit
> -ammend" but it would still be legal to do "git commit -am foo". Your
> patch disallows the latter.

Yes, that syntax looks reasonable. I expect this to be more involved, so I
will rework the patch once we agree on whether or not we want it at all.

> To be honest, I am not sure that even the more restricted proposal is
> that good an idea. You are introducing a heuristic to guess at what is a
> typo or error from the user; when your guess is wrong, the user will be
> annoyed (doubly so if it is buried deep in a script, which this change
> will also impact).

Yes, that can happen. On the other hand, the "-ammend" typo actually did
happen. And what I'm even more worried about are ambiguities like

  $ git commit -uno <path>
  $ git commit -nou <path>

which are interpreted as one of

  $ git commit --untracked-files=no <path>
  $ git commit --untracked-files --no-verify --only <path>

depending only on the order of the switches. I was actually surprised that I
could find an example so easily. But the fact alone that it's possible feels
like an accident waiting to happen.

> On the other hand, the cuddled value already has some DWYM magic (it
> recognizes -amend), so it is already a little bit unsafe to use

Well, an error message is a lot safer than executing something you did not
intend.

> So I don't feel _too_ strongly. I am just concerned that we are
> introducing some DWYM magic that is not really going to help many people
> out, and is just going to make understanding the rules for option
> parsing more complex, and introduce the possibility (albeit slim) of
> breaking people's scripts and trained fingers.

But it makes the rules simpler, because it removes ambiguities such as the
one above.

Yes, we deliberately allow users to shoot themselves in the foot. But they
should have to use at least a long option to do it.

Clemens

^ permalink raw reply

* Re: [PATCH 1/2] do not mangle short options which take arguments
From: Jeff King @ 2009-10-02  6:11 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Shawn O. Pearce, git
In-Reply-To: <20091001201648.GA12175@localhost>

On Thu, Oct 01, 2009 at 10:16:48PM +0200, Clemens Buchacher wrote:

> Instead of
> 
>   $ git commit -a -ammend
>   [work ce38944] mend
>    1 files changed, 2 insertions(+), 0 deletions(-)
> 
> we now get
> 
>   $ git commit -a -ammend
>   error: switch `m' must not be mangled with other options
>   usage: git commit [options] [--] <filepattern>...
>   [...]
> 
> Signed-off-by: Clemens Buchacher <drizzd@aon.at>
> ---
> On Fri, Sep 25, 2009 at 04:32:26PM -0700, Shawn O. Pearce wrote:
> > I wonder, should the -m flag on commit not allow cuddling its
> > value against the switch when its combined in short form with
> > other switches?
> 
> Here we go.

I thought the proposal was to disallow just cuddling of the value when
the switch was combined with others. So you would disallow "git commit
-ammend" but it would still be legal to do "git commit -am foo". Your
patch disallows the latter.

I would prefer to allow the uncuddled form, as it is something I do
occasionally (and I don't think "git commit -am foo" looks very much
like a typo).

To be honest, I am not sure that even the more restricted proposal is
that good an idea. You are introducing a heuristic to guess at what is a
typo or error from the user; when your guess is wrong, the user will be
annoyed (doubly so if it is buried deep in a script, which this change
will also impact). So you are guessing that people don't use the
cuddled form in this way. I suspect most don't. But I also wonder how
many typos you are really helping to save. This was brought up to make
"-ammend" DWYM. Is that really that common a double-typo?

On the other hand, the cuddled value already has some DWYM magic (it
recognizes -amend), so it is already a little bit unsafe to use
(interestingly, though, the gitcli(7) documentation actually recommends
using the "sticked" form over the separated one. However, it also
recommends splitting your short options).

So I don't feel _too_ strongly. I am just concerned that we are
introducing some DWYM magic that is not really going to help many people
out, and is just going to make understanding the rules for option
parsing more complex, and introduce the possibility (albeit slim) of
breaking people's scripts and trained fingers.

-Peff

^ permalink raw reply

* [PATCH] tests: make all test files executable
From: Mark Rada @ 2009-10-02  1:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

For consistency with the rest of the test files.

Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---

On 2009-10-01, at 4:13 AM, Jakub Narebski wrote:
>> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
>> index d0ff21d..0688a57 100644
>> --- a/t/t9501-gitweb-standalone-http-status.sh
>> +++ b/t/t9501-gitweb-standalone-http-status.sh
> 
> BTW. the rest of test scripts are executable, but not this one? Why?
> (But correcting this should be done, if needed, in separate commit).

I noticed one other test script that was not set to be executable.


 0 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 t/t5531-deep-submodule-push.sh
 mode change 100644 => 100755 t/t9501-gitweb-standalone-http-status.sh

diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
old mode 100644
new mode 100755
diff --git a/t/t9501-gitweb-standalone-http-status.sh
b/t/t9501-gitweb-standalone-http-status.sh
old mode 100644
new mode 100755
-- 
1.6.5.rc2

^ permalink raw reply

* [PATCH] filter-branch: add --prune-empty to option summary
From: Adam Brewster @ 2009-10-02  0:52 UTC (permalink / raw)
  To: git; +Cc: Adam Brewster
In-Reply-To: <c376da900910011747i894404dne1ea60dae5e3990b@mail.gmail.com>

Signed-off-by: Adam Brewster <adambrewster@gmail.com>
---
 Documentation/git-filter-branch.txt |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 451950b..2cc3bd8 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -12,6 +12,7 @@ SYNOPSIS
 	[--index-filter <command>] [--parent-filter <command>]
 	[--msg-filter <command>] [--commit-filter <command>]
 	[--tag-name-filter <command>] [--subdirectory-filter <directory>]
+	[--prune-empty]
 	[--original <namespace>] [-d <directory>] [-f | --force]
 	[--] [<rev-list options>...]
 
-- 
1.6.0.6

^ permalink raw reply related

* Re: Trying to split repository
From: Adam Brewster @ 2009-10-02  0:47 UTC (permalink / raw)
  To: Josef Wolf, git
In-Reply-To: <20091001211340.GB26068@raven.wolf.lan>

>>
>> git-filter-branch accepts a --prune-empty option that does what I
>> think you're looking for.
>
> Thanks for your answer, Adam!
>
> Is this a new option? 1.6.0.2 don't seem to have it?

1.6.0.2 was released September 2008 (git log -n1 v1.6.0.2).

This feature was added in October 2008.  (git blame
Documentation/git-filter-branch.txt; git log -n1 d3240d93).

It's still it is missing from the option summary in master though.

Adam

^ permalink raw reply

* Re: [PATCH 2/2] add NORETURN_PTR for function pointers
From: Jeff King @ 2009-10-02  0:43 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: Shawn O. Pearce, git, msysgit, gitster, Erik Faye-Lund
In-Reply-To: <40aa078e0910010957q4daf104cja218af3aa5424637@mail.gmail.com>

On Thu, Oct 01, 2009 at 09:57:12AM -0700, Erik Faye-Lund wrote:

> On Thu, Oct 1, 2009 at 1:17 AM, Jeff King <peff@peff.net> wrote:
> > Thanks, this version and (your 1/2) both look sane to me. The only thing
> > missing are some Makefile knobs to tweak this, but I will assume that
> > will come as part of a later MSVC-compatibility series.
> 
> Thanks for reviewing :)
> 
> I sent an additional patch to the msysgit mailing-list that defines
> NORETURN for MSVC, but I think it's better to keep it out of git.git
> for a little while. There's no Makefile-knobs, it checks for _MSC_VER
> (similar to what's done for GCC).

That sounds good. Thanks.

-Peff

^ permalink raw reply

* stgit, rebasing with 100 patches
From: Jon Smirl @ 2009-10-01 23:04 UTC (permalink / raw)
  To: Git Mailing List

I have 100 patches loaded into in stgit. My tree is at 2.6.30. Now I
want to rebase to 2.6.31-rc1. About 30 of these hundred patches got
committed in this interval.

If I rebase directly to 2.6.31-rc1 I end up with a bunch of merge
conflicts as the patches are applied. That's because patches 'a,b,c'
got applied in the merge window. When I push 'a' back down it sees the
combination of 'a,b,c' not just 'a'. It is unable to figure out that
'a' was applied and then 'b' and 'c' applied on top of it.

Is there a better way to locate the patches the got applied?

-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply


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