* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Torsten Bögershausen @ 2013-01-02 0:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Torsten Bögershausen, git
In-Reply-To: <7v7gnw8slq.fsf@alter.siamese.dyndns.org>
On 01.01.13 23:07, Junio C Hamano wrote:
[snip]
> What it checks looks like a good start, but the indentation of it
> (and the log message) seems very screwed up.
OK
> I also have to wonder what's the false positive rate of this. When
> you are preparing a new test, you would ideally want a mode that
> checks only parts that you just added, without seeing noises from
> existing violations and false positives from the part you did not
> touch. Otherwise, it will be too cumbersome to run for developers,
> and the check mechanism will end up used by nobody.
>
The script found all problems which make the testsuite (unecessary) fail on Mac OS X.
The false positive rate is currently 0% (otherwise I should not have send it to the list)
The suggestion is to run it every time the test suite is run, at the begining.
And it seems to be fast enough:
$ time ./check-non-portable-shell.pl ../../git.master/t/t[0-9]*.sh
real 0m0.263s
user 0m0.239s
sys 0m0.021s
/Torsten
^ permalink raw reply
* Re: [PATCH 1/8] Use %B for Split Subject/Body
From: Junio C Hamano @ 2013-01-02 0:30 UTC (permalink / raw)
To: greened; +Cc: git, Techlive Zheng
In-Reply-To: <87wqvwfsfm.fsf@waller.obbligato.org>
greened@obbligato.org writes:
> Ack, of course. I don't know how I missed that.
>
>>> # 15
>>> test_expect_success 'add main6' '
>>> create main6 &&
>>
>> Why?
>
> It was in the original testsuite from Avery. I didn't add or remove any
> tests when I first integrated git-subtree.
The question was about the lossage of the blank line, which does not
seem to be related to what this patch wants to do.
>>> -# 25
>>> +#25
>>
>> Why the lossage of a SP?
>
> I think this got fixed later in the series.
That is not a good excuse to introduce breakages in the first place, no?
>> It may make sense to lose these "# num" that will have to be touched
>> every time somebody inserts new test pieces in the middle, as a
>> preparatory step before any of these patches, by the way. That will
>> reduce noise in the patches for real changes.
>
> Yeah, I know, but it makes it really easy to find a test when something
> goes wrong.
That is what "tXXXX-*.sh -i" is for, isn't it?
^ permalink raw reply
* Re: [PATCH 2/8] Add --unannotate
From: Junio C Hamano @ 2013-01-02 0:32 UTC (permalink / raw)
To: greened; +Cc: git, James Nylen
In-Reply-To: <87sj6kfsbz.fsf@waller.obbligato.org>
greened@obbligato.org writes:
> In the meantime, will you apply the patch or do you prefer a new design?
The --unannotate option will become a baggage you will have to keep
working until the end of time, if we applied it. I think it is not
too uch a baggage, so it probably is OK.
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Eric S. Raymond @ 2013-01-02 0:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfw2k8t7k.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> So..., is this a flag-day patch?
>
> After this is merged, users who have been interoperating with CVS
> repositories with the older cvsps have to install the updated cvsps
> before using a new version of Git that ships with it?
Yes, they must install an updated cvsps. But this is hardly a loss, as
the old version was perilously broken.
There was an error or typo in the branch-analysis code, dating from
2006 and possibly earlier, that meant that branch root points would
almost always be attributed to parent patchsets one patchset earlier
than they should have been. Shocked me when I found it - how was this
missed for six years?
Because of the way the analysis is done, this fundamental bug would
also cause secondary damage like file changes near the root point
getting attributed to the wrong branch. In fact, this is how I
first spotted the problem; my test suite exhibited this symptom.
And mind you this is on top of ancestry-branch tracking not working -
two separate bugs that could interact in ways I'd really rather not
think about. The bottom line is that every import of a branchy CVS
repo with a pre-3.x version of cvsps is probably wrong.
The old git-cvsimport code was doing its part to screw things up, too.
At least three of the bugs on its manual page are problems I couldn't
reproduce using a bare cvsps instance, even the old broken version.
> As long as
> they update both cvsps and cvsimport, they can continue using the
> existing repository to get updates from the same upstream CVS
> repository without losing hisory continuity?
Yes, but in that case I would strongly advise re-importing the entire
CVS history, as the portion analyzed with 2.2b1 and earlier versions
of cvsps will almost certainly have been somewhat garbled if it
contains any branches.
> I would have preferred an addition of "git cvsimport-new" (or rename
> of the existing one to "git cvsimport-old"), with additional tests
> that compare the results of these two implemenations on simple CVS
> history that cvsimport-old did *not* screw up, to ensure that (1)
> people with existing set-up can choose to keep using the old one,
> perhaps by tweaking their process to use cvsimport-old, and (2) the
> updated one will give these people the identical conversion results,
> as long as the history they have been interacting with do not have
> the corner cases that trigger bugs in older cvsps.
>
> Or am I being too conservative?
I think you are being too conservative. This patch is *not* a mere
feature upgrade. The branch-analysis bug I found three days ago is not
a minor problem, it is a big ugly showstopper for any case beside the
simplest linear histories. Only linear histories will not break.
'People with existing set-ups' should absolutely *not* 'keep using the
old one'; we should yank that choice away from them and get the old
cvsimport/cvsps pair out of use *as fast as possible*, because it
silently mangles branchy imports.
Accordingly, giving people the idea that it's OK to use old and new
versions in parallel would be an extremely bad idea. I would go so
far as to call it irresponsible.
Here is what I have done to ease the transition:
If you try to use old git-cvsimport with new cvsps, new cvsps will detect
this and ship a message to stderr telling you to upgrade
If you try to use new git-cvsimport with old cvsps, old cvsps will complain
of an invalid argument and git-cvsimport will quit.
As for testing...cvsps now has several dozen self-tests on five
different CVS repositories, including improved versions of the
t960[123] tests. I will keep developing these as I work on bringing
parsecvs up to snuff.
I don't think there is a lot of point in git-cvsimport having its own
tests any more. If you read it I think you'll see why; it's a much
thinner wrapper around the conversion engine(s) than it used to be. In
particular, it no longer does its own protocol transactions to the
CVS server.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Junio C Hamano @ 2013-01-02 1:06 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20130102003344.GA9651@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> Junio C Hamano <gitster@pobox.com>:
>> So..., is this a flag-day patch?
>>
>> After this is merged, users who have been interoperating with CVS
>> repositories with the older cvsps have to install the updated cvsps
>> before using a new version of Git that ships with it?
>
> Yes, they must install an updated cvsps. But this is hardly a loss, as
> the old version was perilously broken.
>
> There was an error or typo in the branch-analysis code, dating from
> 2006 and possibly earlier, that meant that branch root points would
> almost always be attributed to parent patchsets one patchset earlier
> than they should have been. Shocked me when I found it - how was this
> missed for six years?
Would it be that not many people use branchy history in CVS, as
merging there was soooo painful?
>> Or am I being too conservative?
>
> I think you are being too conservative. This patch is *not* a mere
> feature upgrade. The branch-analysis bug I found three days ago is not
> a minor problem, it is a big ugly showstopper for any case beside the
> simplest linear histories. Only linear histories will not break.
That is exactly my point. It never worked in a branchy history, and
that is an indication that people who didn't complain and used the
old cvsimport with branch-incapable cvsps happily would have been
working with a linear history. Either nobody uses cvsimport in the
daily work, in which case a flag-day is perfectly fine, or we will
have many people who are forced to update to unproven version for no
immediate upside because the upstream repositories they work with, or
options they use cvsimport, do not trigger the multi-branch bug.
I however do understand that updating is the only sensible thing to
do for them *in the longer term*, as older cvsimport and cvsps are
no longer maintained, and sooner they update the better the chance
the new cvsimport becomes perfect earlier.
> 'People with existing set-ups' should absolutely *not* 'keep using the
> old one'; we should yank that choice away from them and get the old
> cvsimport/cvsps pair out of use *as fast as possible*, because it
> silently mangles branchy imports.
I still am not convinced, especially without a "we make sure we do
not regress in linear histories" side-by-side test in place. That
sounds irresponsible.
But others may disagree, and I'd have to sleep on it.
I'd prefer to hear from somebody who is *not* defending on his newer
implementation, but from somebody who is actively using cvsimport as
an end user. On the end-users' side, there always is this anxiety
that a radical rewrite will always introduce new bugs, even when
they know the rewrite is done very competently.
> Here is what I have done to ease the transition:
>
> If you try to use old git-cvsimport with new cvsps, new cvsps will detect
> this and ship a message to stderr telling you to upgrade
Sounds sensible.
> If you try to use new git-cvsimport with old cvsps, old cvsps will complain
> of an invalid argument and git-cvsimport will quit.
With an error message that tells the user to update cvsps, this also
sounds sensible.
^ permalink raw reply
* Makefile dependency from 'configure' to 'GIT-VERSION-FILE'
From: Martin von Zweigbergk @ 2013-01-02 1:11 UTC (permalink / raw)
To: Stefano Lattarini, Jeff King, Jonathan Nieder; +Cc: git
Hi,
I use autoconf with git.git. I have noticed lately, especially when
doing things like "git rebase -i --exec make", that ./configure is run
every time. If I understand correctly, this is because of 8242ff4
(build: reconfigure automatically if configure.ac changes,
2012-07-19). Just a few days before that commit, on 2012-07-15, the
branch jn/makefile-cleanup including 520a6cd (Makefile: move
GIT-VERSION-FILE dependencies closer to use, 2012-06-20) was merged
(to next?). I wonder if these two subjects were aware of each other.
The reason 'configure' depends on GIT-VERSION-FILE is because it
inserts the version into the call to AC_INIT. I have close to no
experience with autoconf or even make and it's not at all clear to me
why we need to pass the verison to AC_INIT. It seems like it's just
for messages printed by ./configure. If that's the case, we shouldn't
need to generate a new 'configure' file ever time. At the very least,
we shouldn't need to run it.
Do you think we should simply remove the dependency from 'configure'
to 'GIT-VERSION-FILE' and leave a comment there instead? Or should we
instead somehow make 'reconfigure' depend only on 'configure.ac'? Both
of these feel a little wrong to me, because they would remove real
dependencies. Maybe the (probably mangled) patch at the end of this
message is better?
Martin
diff --git a/Makefile b/Makefile
index 736ecd4..ec5d7ca 100644
--- a/Makefile
+++ b/Makefile
@@ -2267,12 +2267,9 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
mv $@+ $@
endif # NO_PYTHON
-configure: configure.ac GIT-VERSION-FILE
- $(QUIET_GEN)$(RM) $@ $<+ && \
- sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
- $< > $<+ && \
- autoconf -o $@ $<+ && \
- $(RM) $<+
+configure: configure.ac
+ $(QUIET_GEN)$(RM) $@ && \
+ autoconf -o $@ $<
ifdef AUTOCONFIGURED
config.status: configure
diff --git a/configure.ac b/configure.ac
index ad215cc..00c3e38 100644
--- a/configure.ac
+++ b/configure.ac
@@ -142,7 +142,10 @@ fi
## Configure body starts here.
AC_PREREQ(2.59)
-AC_INIT([git], [@@GIT_VERSION@@], [git@vger.kernel.org])
+AC_INIT([git],
+ m4_esyscmd([ ./GIT-VERSION-GEN &&
+ { sed -ne 's/GIT_VERSION = //p' GIT-VERSION-FILE
| xargs echo -n; } ]),
+ [git@vger.kernel.org])
AC_CONFIG_SRCDIR([git.c])
^ permalink raw reply related
* Re: [PATCH 3/4] t4014: do not uese echo -n
From: Junio C Hamano @ 2013-01-02 1:16 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, Torsten Bögershausen
In-Reply-To: <201301012241.17050.tboegi@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> echo -n is not portable on all systems.
> Use printf instead
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
Brandon, this comes from 932581b (Unify appending signoff in
format-patch, commit and sequencer, 2012-11-25). Please make sure
to squash it in when you reroll the series.
Thanks (and a happy new year ;-).
> t/t4014-format-patch.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 6cfad13..f460930 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -993,7 +993,7 @@ EOF
> '
>
> test_expect_success 'signoff: commit with only subject that does not end with NL' '
> - echo -n subject | append_signoff >actual &&
> + printf subject | append_signoff >actual &&
> cat >expected <<\EOF &&
> 4:Subject: [PATCH] subject
> 8:
^ permalink raw reply
* Re: [PATCH 2/3] format-patch: pick up branch description when no ref is specified
From: Duy Nguyen @ 2013-01-02 1:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v38ykbpv3.fsf@alter.siamese.dyndns.org>
On Wed, Jan 2, 2013 at 3:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> find_branch_name() fails to detect "format-patch --cover-letter -3"
>> where no command line arguments are given and HEAD is automatically
>> added.
>
> Nicely spotted.
>
> That is not the only case that takes this codepath, though.
>
> $ git format-patch --cover-letter master..
>
> will also give you the same (if you say it without "..", which is
> the more normal invocation of the command, then the caller already
> know you meant the current branch and this function is not called).
>
> And in that case you will have two tokens on cmdline.nr, one for
> "master.." to show where he bottom is, and the other for the
> implied "HEAD";
Interesting. find_brach_name() handles this case wrong because
rev->cmdline[positive].name is "HEAD" and it goes ahead prepending
"refs/heads/" anyway. I'll fix it in the reroll. I was avoiding tests
with an excuse that you did not write tests when you added this
function either. But with this change, I think tests are required.
> I do not think this patch is a sufficient solution
> for the more general cases, but on the other hand I do not know how
> much it matters.
I think the best place to handle this is setup_revisions() for general
cases. But we do not need branch detection anywhere else yet, I guess
it's ok to fix it case by case here. We can move the code back to
revisions.c when there are more use cases for it.
>> - if (positive < 0)
>> + if (positive < 0) {
>> + /*
>> + * No actual ref from command line, but "HEAD" from
>> + * rev->def was added in setup_revisions()
>> + * e.g. format-patch --cover-letter -12
>> + */
>
> That comment does not describe "positive < 0" case, but belongs to
> the conditional added in this patch, no?
It's meant as the comment for the following block, yes. I'll move it
into the block for clarity.
>> + if (!rev->cmdline.nr &&
>> + rev->pending.nr == 1 &&
>> + !strcmp(rev->pending.objects[0].name, "HEAD")) {
>> + const char *ref;
>> + ref = resolve_ref_unsafe("HEAD", branch_sha1, 1, NULL);
>> + if (ref && !prefixcmp(ref, "refs/heads/"))
>> + return xstrdup(ref + strlen("refs/heads/"));
>> + }
>> return NULL;
>> + }
>> strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
>> branch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL);
>> if (!branch ||
--
Duy
^ permalink raw reply
* Re: Bug in latest gitk - can't click lines connecting commits
From: Paul Mackerras @ 2013-01-01 23:22 UTC (permalink / raw)
To: Stefan Haller; +Cc: Jason Holden, git
In-Reply-To: <1kw18d3.5sftkl125qdz4M%stefan@haller-berlin.de>
On Tue, Jan 01, 2013 at 06:54:23PM +0100, Stefan Haller wrote:
> Jason Holden <jason.k.holden.swdev@gmail.com> wrote:
>
> > I was testing some patches against the latest gitk, and noticed that when I
> > click the mouse on the lines that connect the commits in the history graph,
> > I get an error popup with:
> > Error: can't read "cflist_top": no such variable
> >
> > Looks like this was introduced in gitk commit b967135d89e8d8461d059
> > gitk: Synchronize highlighting in file view when scrolling diff
>
> A patch that fixes this was proposed over two months ago, and Paul said
> he had applied it:
>
> <http://permalink.gmane.org/gmane.comp.version-control.git/208162>
>
> However, looking at git://ozlabs.org/~paulus/gitk.git it's not there.
> Paul?
I just forgot to push it out. It's there now.
Paul.
^ permalink raw reply
* [PATCH v3] git-clean: Display more accurate delete messages
From: Zoltan Klinger @ 2013-01-02 1:45 UTC (permalink / raw)
To: git; +Cc: Zoltan Klinger
(1) Only print out the names of the files and directories that got
actually deleted.
(2) Show warning message for ignored untracked git repositories
Consider the following repo layout:
test.git/
|-- tracked_dir/
| |-- some_tracked_file
| |-- some_untracked_file
|-- tracked_file
|-- untracked_file
|-- untracked_foo/
| |-- bar/
| | |-- bar.txt
| |-- emptydir/
| |-- frotz.git/
| |-- frotz.tx
|-- untracked_some.git/
|-- some.txt
Suppose the user issues 'git clean -fd' from the test.git directory.
When -d option is used and untracked directory 'foo' contains a
subdirectory 'frotz.git' that is managed by a different git repository
therefore it will not be removed.
$ git clean -fd
Removing tracked_dir/some_untracked_file
Removing untracked_file
Removing untracked_foo/
Removing untracked_some.git/
The message displayed to the user is slightly misleading. The foo/
directory has not been removed because of foo/frotz.git still exists.
On the other hand the subdirectories 'bar' and 'emptydir' have been
deleted but they're not mentioned anywhere. Also, untracked_some.git
has not been removed either.
This behaviour is the result of the way the deletion of untracked
directories are reported. In the current implementation they are
deleted recursively but only the name of the top most directory is
printed out. The calling function does not know about any
subdirectories that could not be removed during the recursion.
Improve the way the deleted directories are reported back to
the user:
(1) Create a recursive delete function 'remove_dirs' in builtin/clean.c
to run in both dry_run and delete modes with the delete logic as
follows:
(a) Check if the current directory to be deleted is an untracked
git repository. If it is and --force --force option is not set
do not touch this directory, print ignore message, set dir_gone
flag to false for the caller and return.
(b) Otherwise for each item in current directory:
(i) If current directory cannot be accessed, print warning,
set dir_gone flag to false and return.
(ii) If the item is a subdirectory recurse into it,
check for the returned value of the dir_gone flag.
If the subdirectory is gone, add the name of the deleted
directory to a list of successfully removed items 'dels'.
Else set the dir_gone flag as the current directory
cannot be removed because we have at least one subdirectory
hanging around.
(iii) If it is a file try to remove it. If success add the
file name to the 'dels' list, else print error and set
dir_gone flag to false.
(c) After we finished deleting all items in the current directory and
the dir_gone flag is still true, remove the directory itself.
If failed set the dir_gone flag to false.
(d) If the current directory cannot be deleted because the dir_gone flag
has been set to false, print out all the successfully deleted items
for this directory from the 'dels' list.
(e) We're done with the current directory, return.
(2) Modify the cmd_clean() function to:
(a) call the recursive delete function 'remove_dirs()' for each
topmost directory it wants to remove
(b) check for the returned value of dir_gone flag. If it's true
print the name of the directory as being removed.
Consider the output of the improved version:
$ git clean -fd
Removing tracked_dir/some_untracked_file
Removing untracked_file
warning: ignoring untracked git repository untracked_foo/frotz.git
Removing untracked_foo/bar
Removing untracked_foo/emptydir
warning: ignoring untracked git repository untracked_some.git/
Now it displays only the file and directory names that got actually
deleted and shows warnings about ignored untracked git repositories.
Reported-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
Signed-off-by: Zoltan Klinger <zoltan.klinger@gmail.com>
---
builtin/clean.c | 149 ++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 120 insertions(+), 29 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 69c1cda..37e403a 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -10,6 +10,7 @@
#include "cache.h"
#include "dir.h"
#include "parse-options.h"
+#include "refs.h"
#include "string-list.h"
#include "quote.h"
@@ -20,6 +21,13 @@ static const char *const builtin_clean_usage[] = {
NULL
};
+static const char* MSG_REMOVE = "Removing %s\n";
+static const char* MSG_WOULD_REMOVE = "Would remove %s\n";
+static const char* MSG_WOULD_NOT_REMOVE = "Would not remove %s\n";
+static const char* MSG_WOULD_IGNORE_GIT_DIR = "Would ignore untracked git repository %s\n";
+static const char* MSG_WARN_GIT_DIR_IGNORE = "ignoring untracked git repository %s";
+static const char* MSG_WARN_REMOVE_FAILED = "failed to remove %s";
+
static int git_clean_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "clean.requireforce"))
@@ -34,11 +42,109 @@ static int exclude_cb(const struct option *opt, const char *arg, int unset)
return 0;
}
+static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
+ int dry_run, int quiet, int *dir_gone)
+{
+ DIR *dir;
+ struct strbuf quoted = STRBUF_INIT;
+ struct dirent *e;
+ int res = 0, ret = 0, gone = 1, original_len = path->len, len, i;
+ unsigned char submodule_head[20];
+ struct string_list dels = STRING_LIST_INIT_DUP;
+
+ *dir_gone = 1;
+
+ quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
+ if ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
+ !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
+ if (dry_run && !quiet)
+ printf(_(MSG_WOULD_IGNORE_GIT_DIR), quoted.buf);
+ else if (!dry_run)
+ warning(_(MSG_WARN_GIT_DIR_IGNORE), quoted.buf);
+
+ *dir_gone = 0;
+ return 0;
+ }
+
+ dir = opendir(path->buf);
+ if (!dir) {
+ /* an empty dir could be removed even if it is unreadble */
+ res = dry_run ? 0 : rmdir(path->buf);
+ if (res) {
+ warning(_(MSG_WARN_REMOVE_FAILED), quoted.buf);
+ *dir_gone = 0;
+ }
+ return res;
+ }
+
+ if (path->buf[original_len - 1] != '/')
+ strbuf_addch(path, '/');
+
+ len = path->len;
+ while ((e = readdir(dir)) != NULL) {
+ struct stat st;
+ if (is_dot_or_dotdot(e->d_name))
+ continue;
+
+ strbuf_setlen(path, len);
+ strbuf_addstr(path, e->d_name);
+ quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
+ if (lstat(path->buf, &st))
+ ; /* fall thru */
+ else if (S_ISDIR(st.st_mode)) {
+ if (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))
+ ret = 1;
+ if (gone)
+ string_list_append(&dels, quoted.buf);
+ else
+ *dir_gone = 0;
+ continue;
+ } else {
+ res = dry_run ? 0 : unlink(path->buf);
+ if (!res)
+ string_list_append(&dels, quoted.buf);
+ else {
+ warning(_(MSG_WARN_REMOVE_FAILED), quoted.buf);
+ *dir_gone = 0;
+ ret = 1;
+ }
+ continue;
+ }
+
+ /* path too long, stat fails, or non-directory still exists */
+ *dir_gone = 0;
+ ret = 1;
+ break;
+ }
+ closedir(dir);
+
+ strbuf_setlen(path, original_len);
+ quote_path_relative(path->buf, strlen(path->buf), "ed, prefix);
+
+ if (*dir_gone) {
+ res = dry_run ? 0 : rmdir(path->buf);
+ if (!res)
+ *dir_gone = 1;
+ else {
+ warning(_(MSG_WARN_REMOVE_FAILED), quoted.buf);
+ *dir_gone = 0;
+ ret = 1;
+ }
+ }
+
+ if (!*dir_gone && !quiet) {
+ for (i = 0; i < dels.nr; i++)
+ printf(dry_run ? _(MSG_WOULD_REMOVE) : _(MSG_REMOVE), dels.items[i].string);
+ }
+ string_list_clear(&dels, 0);
+ return ret;
+}
+
int cmd_clean(int argc, const char **argv, const char *prefix)
{
- int i;
- int show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;
- int ignored_only = 0, config_set = 0, errors = 0;
+ int i, res;
+ int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;
+ int ignored_only = 0, config_set = 0, errors = 0, gone = 1;
int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;
struct strbuf directory = STRBUF_INIT;
struct dir_struct dir;
@@ -49,7 +155,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
char *seen = NULL;
struct option options[] = {
OPT__QUIET(&quiet, N_("do not print names of files removed")),
- OPT__DRY_RUN(&show_only, N_("dry run")),
+ OPT__DRY_RUN(&dry_run, N_("dry run")),
OPT__FORCE(&force, N_("force")),
OPT_BOOLEAN('d', NULL, &remove_directories,
N_("remove whole directories")),
@@ -77,7 +183,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
if (ignored && ignored_only)
die(_("-x and -X cannot be used together"));
- if (!show_only && !force) {
+ if (!dry_run && !force) {
if (config_set)
die(_("clean.requireForce set to true and neither -n nor -f given; "
"refusing to clean"));
@@ -150,38 +256,23 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
if (S_ISDIR(st.st_mode)) {
strbuf_addstr(&directory, ent->name);
qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);
- if (show_only && (remove_directories ||
- (matches == MATCHED_EXACTLY))) {
- printf(_("Would remove %s\n"), qname);
- } else if (remove_directories ||
- (matches == MATCHED_EXACTLY)) {
- if (!quiet)
- printf(_("Removing %s\n"), qname);
- if (remove_dir_recursively(&directory,
- rm_flags) != 0) {
- warning(_("failed to remove %s"), qname);
+ if (remove_directories || (matches == MATCHED_EXACTLY)) {
+ if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))
errors++;
- }
- } else if (show_only) {
- printf(_("Would not remove %s\n"), qname);
- } else {
- printf(_("Not removing %s\n"), qname);
+ if (gone && !quiet)
+ printf(dry_run ? _(MSG_WOULD_REMOVE) : _(MSG_REMOVE), qname);
}
strbuf_reset(&directory);
} else {
if (pathspec && !matches)
continue;
qname = quote_path_relative(ent->name, -1, &buf, prefix);
- if (show_only) {
- printf(_("Would remove %s\n"), qname);
- continue;
- } else if (!quiet) {
- printf(_("Removing %s\n"), qname);
- }
- if (unlink(ent->name) != 0) {
- warning(_("failed to remove %s"), qname);
+ res = dry_run ? 0 : unlink(ent->name);
+ if (res) {
+ warning(_(MSG_WARN_REMOVE_FAILED), qname);
errors++;
- }
+ } else if (!quiet)
+ printf(dry_run ? _(MSG_WOULD_REMOVE) :_(MSG_REMOVE), qname);
}
}
free(seen);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories
From: Jason Holden @ 2013-01-02 1:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1357082695-29713-3-git-send-email-gitster@pobox.com>
On Tue, Jan 01, 2013 at 03:24:54PM -0800, Junio C Hamano wrote:
>
> ------------------------------------------------
> +Subsystems with dedicated maintainers
> +
> +Some parts of the system have dedicated maintainers with their own
> +repositories.
> +
> + - git-gui/ comes from git-gui project, maintained by Pat Thoyts:
> +
> + git://repo.or.cz/git-gui.git
> +
> + - gitk-git/ comes from Paul Mackerras's gitk project:
> +
> + git://ozlabs.org/~paulus/gitk
> +
> + - po/ comes from the localization coordinator, Jiang Xin:
> +
> + https://github.com/git-l10n/git-po/
> +
> +Patches to these parts should be based on their trees.
> +
> +------------------------------------------------
> An ideal patch flow
>
Any reason to leave out the maintainers email addresses? If we add that, all
the content of the MAINTAINERS file we had been discussing would then be in
this file.
My only other suggestion for this series might be to augment the file with
a patch submittal example(s). I found the best example to be in
git-send-email's man page, but maybe enumerate the example out for the
most common workflows. Maybe something like:
-----------------------------------------------------------
Example of sending patches for a new feature:
Create the patches:
$ git format-patch --cover-letter -M origin/master -o outgoing/
$ edit outgoing/0000-*
To send your completed patches for initial consideration:
$ git send-email outgoing/* -to git@vger.kernel.org -cc gitster@pobox.com
To send your reviewed patches for inclusion:
$ git send-email outgoing/* -to gitster@pobox.com -cc git@vger.kernel.org
-Jason
^ permalink raw reply
* Re: [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories
From: Junio C Hamano @ 2013-01-02 2:14 UTC (permalink / raw)
To: Jason Holden; +Cc: git
In-Reply-To: <20130102015233.GA25288@gmail.com>
Jason Holden <jason.k.holden.swdev@gmail.com> writes:
> Any reason to leave out the maintainers email addresses?
Nothing particular, other than that I did not find anywhere in the
file that does not break the flow.
> My only other suggestion for this series might be to augment the file with
> a patch submittal example(s). I found the best example to be in
> git-send-email's man page,...
I'd feel better to avoid copying and pasting. If send-email has
good examples, shouldn't it be sufficient to refer to them?
> -----------------------------------------------------------
> Example of sending patches for a new feature:
>
> Create the patches:
> $ git format-patch --cover-letter -M origin/master -o outgoing/
> $ edit outgoing/0000-*
>
> To send your completed patches for initial consideration:
> $ git send-email outgoing/* -to git@vger.kernel.org -cc gitster@pobox.com
This is ambiguous; it makes it look as if you are CC'ing the
maintainer, but for the initial round, it is likely that you are
sending it to me only because I have been involved in the area the
patch touches.
> To send your reviewed patches for inclusion:
> $ git send-email outgoing/* -to gitster@pobox.com -cc git@vger.kernel.org
This is fine, but we would probably want to see it CC'ed to people
who reviewed the initial submission, too.
^ permalink raw reply
* Re: [PATCH 3/8] Better Error Handling for add
From: Junio C Hamano @ 2013-01-02 2:21 UTC (permalink / raw)
To: greened; +Cc: git
In-Reply-To: <87obh8fs8e.fsf@waller.obbligato.org>
greened@obbligato.org writes:
>> If you want to make sure you give a comit to add_commit, you can
>> probably say something like this:
>>
>> git rev-parse -q --verify "$1^{commit}" >/dev/null ||
>> die "'$1' does not refer to a commit"
>
> What does $1^{commit} mean?
"$thing^{type}" tells Git to interpret the $thing as that type (and
error out if it can't).
So v1.0.0^{commit} is a less cryptic way to say v1.0.0^0 (there is
no need to say "zeroth parent of a commit is the commit itself?
Yeah, it makes sort of sense" when you learn it).
"git cat-file -t junio-gpg-pub^{blob}" will say "blob", but you will
get a failure from "git rev-parse v1.0.0^{blob}" as you can only
dereference a tag that refers to a commit down to the comit and then
to its top-level tree, but not to a single blob.
And you can ask for the tree object with v1.0.0^{tree}, for example.
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Junio C Hamano @ 2013-01-02 2:22 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <50E37BD3.6060709@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> The suggestion is to run it every time the test suite is run, at the begining.
> And it seems to be fast enough:
>
> $ time ./check-non-portable-shell.pl ../../git.master/t/t[0-9]*.sh
> real 0m0.263s
> user 0m0.239s
> sys 0m0.021s
Hmph. OK.
^ permalink raw reply
* Re: [RFC] pack-objects: compression level for non-blobs
From: Duy Nguyen @ 2013-01-02 2:23 UTC (permalink / raw)
To: Shawn Pearce
Cc: Jeff King, David Michael Barr, Git Mailing List, Junio C Hamano
In-Reply-To: <CAJo=hJsZedd0kfYJnXPhcud8bz3mgU0NMf6O6-_PY1yqv-EfDg@mail.gmail.com>
On Wed, Jan 2, 2013 at 12:17 AM, Shawn Pearce <spearce@spearce.org> wrote:
>> And I was wrong. At least since 1b4bb16 (pack-objects: optimize
>> "recency order" - 2011-06-30) commits are spread out and can be mixed
>> with trees too. Grouping them back defeats what Junio did in that
>> commit, I think.
>
> I think you misunderstand what 1b4bb16 does. Junio uses a layout
> similar to what JGit has done for years. Commits are packed, then
> trees, then blobs. Only annotated tags are interspersed with commits.
> The decision on where to place tags is different, but has a similar
> purpose.
This is embarrassing. I looked at verify-pack output and somehow saw
trees mixed with commits. I must have read it wrong. "git verify-pack
-v <pack>|awk '{print $2;}'|uniq on recently created pack shows that
only tags and commits are mixed. Sorry for the noise.
--
Duy
^ permalink raw reply
* Test failures with python versions when building git 1.8.1
From: Dan McGee @ 2013-01-02 4:12 UTC (permalink / raw)
To: GIT Mailing-list, Florian Achleitner, David Michael Barr
A test case snuck in this release that assumes /usr/bin/python is
python2 and causes test failures. Unlike all other tests and code
depending on python, this one does not respect PYTHON_PATH, which we
explicitly set when building git on Arch Linux due to python2 vs
python3 differences.
-Dan
make[1]: Entering directory `/build/src/git-1.8.1/t'
rm -f -r test-results
*** prove ***
Test Summary Report
-------------------
t9020-remote-svn.sh (Wstat: 256 Tests: 6 Failed: 4)
Failed tests: 1-2, 5-6
Non-zero exit status: 1
Files=608, Tests=8772, 76 wallclock secs ( 4.07 usr 0.65 sys + 91.83
cusr 37.14 csys = 133.69 CPU)
Result: FAIL
make[1]: *** [prove] Error 1
make[1]: Leaving directory `/build/src/git-1.8.1/t'
make: *** [test] Error 2
$ contrib/svn-fe/svnrdump_sim.py
File "contrib/svn-fe/svnrdump_sim.py", line 43
print "usage: %s dump URL -rLOWER:UPPER"
^
SyntaxError: invalid syntax
diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 1cfac4a..7e6148d 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -40,7 +40,7 @@ def writedump(url, lower, upper):
if __name__ == "__main__":
if not (len(sys.argv) in (3, 4, 5)):
- print "usage: %s dump URL -rLOWER:UPPER"
+ print("usage: %s dump URL -rLOWER:UPPER")
sys.exit(1)
if not sys.argv[1] == 'dump': raise NotImplementedError('only
"dump" is suppported.')
url = sys.argv[2]
^ permalink raw reply related
* Re: Test failures with python versions when building git 1.8.1
From: Junio C Hamano @ 2013-01-02 5:19 UTC (permalink / raw)
To: Dan McGee
Cc: GIT Mailing-list, Florian Achleitner, David Michael Barr,
Eric S. Raymond
In-Reply-To: <CAEik5nOqge8ix4WGf-h+0Dmz1CanH_XtQdB-CxvPsggSu1-LzQ@mail.gmail.com>
Dan McGee <dan@archlinux.org> writes:
> A test case snuck in this release that assumes /usr/bin/python is
> python2 and causes test failures. Unlike all other tests and code
> depending on python, this one does not respect PYTHON_PATH, which we
> explicitly set when building git on Arch Linux due to python2 vs
> python3 differences.
I had an impression that you are not supposed to run our scripts
with python3 yet (no python scripts have been checked for python3
compatibility), even though some people have expressed interests in
doing so.
Eric?
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Eric S. Raymond @ 2013-01-02 5:45 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dan McGee, GIT Mailing-list, Florian Achleitner,
David Michael Barr
In-Reply-To: <7va9ss5fhq.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com>:
> Dan McGee <dan@archlinux.org> writes:
>
> > A test case snuck in this release that assumes /usr/bin/python is
> > python2 and causes test failures. Unlike all other tests and code
> > depending on python, this one does not respect PYTHON_PATH, which we
> > explicitly set when building git on Arch Linux due to python2 vs
> > python3 differences.
>
> I had an impression that you are not supposed to run our scripts
> with python3 yet (no python scripts have been checked for python3
> compatibility), even though some people have expressed interests in
> doing so.
>
> Eric?
Yeah, git's stuff is nowhere even *near* python3 ready.
I have it on my to-do list to run 2to3 on the in-tree scripts as a
diagnostic, but I haven't had time to do that yet...mainly because
cvsps/cvsimport blew up in my face when I poked at them.
Now I need to go beat parsecvs into shape and run both it and cvs2git
against the CVS torture tests I'm developing, so the 2to3 check won't
happen for a week or two at least. Sorry.
As in a general thing, I wouldn't advise worrying too much about python3
compatibility. That version is not gaining adoption very fast, mainly
due to the rat's nest around plain strings vs. UTF-8 which can make
code conversion a serious pain in the ass.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Jeff King @ 2013-01-02 6:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dan McGee, GIT Mailing-list, Florian Achleitner,
David Michael Barr, Eric S. Raymond
In-Reply-To: <7va9ss5fhq.fsf@alter.siamese.dyndns.org>
On Tue, Jan 01, 2013 at 09:19:13PM -0800, Junio C Hamano wrote:
> Dan McGee <dan@archlinux.org> writes:
>
> > A test case snuck in this release that assumes /usr/bin/python is
> > python2 and causes test failures. Unlike all other tests and code
> > depending on python, this one does not respect PYTHON_PATH, which we
> > explicitly set when building git on Arch Linux due to python2 vs
> > python3 differences.
>
> I had an impression that you are not supposed to run our scripts
> with python3 yet (no python scripts have been checked for python3
> compatibility), even though some people have expressed interests in
> doing so.
>
> Eric?
Yeah, but the worrying thing to me is that we are not respecting
PYTHON_PATH. So even if Arch does everything right, it is getting
punished just for having python3 on the system at all.
I think we need to either:
1. Build contrib/svn-fe/svnrdump_sim.py into svnrdump using our normal
build procedure, which handles $PYTHON_PATH (right now we seem to
just symlink[1] it into place as svnrdump during the test script
run).
2. Create svnrdump as a shell script in the test suite to do something
like[2]:
$PYTHON_PATH "$TEST_DIRECTORY"/../contrib/svn-fe/svnrdump_sim.py
-Peff
[1] This symlink is doubly wrong, because any use of symbolic links
in the test scripts needs to depend on the SYMLINKS prereq, and this
does not.
[2] In both the current code and what I showed above, the test scripts
depend on things in contrib/. This is probably a bad idea in
general, as the quality of what goes into contrib is not as closely
watched (especially with respect to things like portability).
Certainly I would not have known to look more carefully at a patch
to contrib/svn-fe for breakage to the test suite.
FWIW, we also have this situation with t9902 (bash completion),
though the dependency is a little more obvious there. It might be
worth promoting bash completion out of contrib (or alternatively,
demoting t9902 into contrib/completion/, possibly with a feature to
make it easier to run tests out of contrib).
^ permalink raw reply
* Re: [PATCH] gitk tag delete/rename support
From: Paul Mackerras @ 2013-01-02 7:11 UTC (permalink / raw)
To: Leon KUKOVEC; +Cc: git
In-Reply-To: <1353870345-3054-1-git-send-email-leon.kukovec@gmail.com>
On Sun, Nov 25, 2012 at 08:05:45PM +0100, Leon KUKOVEC wrote:
> Right clicking on a tag pops up a menu, which allows
> tag to be renamed or deleted.
Nice idea, but I am concerned that renaming a tag that refers to a tag
object will turn it into a lightweight tag, which would be surprising
for users. I think that needs to be fixed before I apply the patch.
Also, when renaming a tag it would be good to check for the cases
where the new name is the same as the old (in which case nothing needs
to be done) and where the new name already exists.
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH] gitk: add a checkbox to control the visibility of tags
From: Paul Mackerras @ 2013-01-02 7:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Łukasz Stelmach
In-Reply-To: <7vlidhmc5i.fsf@alter.siamese.dyndns.org>
On Sat, Dec 01, 2012 at 06:16:25PM -0800, Junio C Hamano wrote:
> Łukasz Stelmach <stlman@poczta.fm> writes:
>
> > Enable hiding of tags displayed in the tree as yellow labels.
> > If a repository is used together with a system like Gerrit
> > there may be quite a lot of tags used to control building
> > and there may be hardly any place left for commit subjects.
> >
> > Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
> > ---
>
> Paul, this patch is not done against your tree (does not have gitk
> at the top-level), but other than that, the change mimics the way
> existing hideremoes is implemented and looks reasonable to me.
>
> We _may_ want to unify these two "hidestuff" into a list of patterns
> that hides any ref that match one of the patterns in the list, e.g.
>
> set hidestuff {refs/heads/*/* refs/tags/* refs/remotes/*}
>
> may hide all tags, all remote-tracking branches and local branches
> that have a slash in their names.
If the concern is the amount of screen real-estate that the tags take
up when there are many of them (which is a reasonable concern), I'd
rather just put a single tag icon with "tags..." inside it and arrange
to list all the tags in the diff display pane when the user clicks on
it. I think that would be better than not showing the tags at all.
Paul.
^ permalink raw reply
* Re: Test failures with python versions when building git 1.8.1
From: Junio C Hamano @ 2013-01-02 7:18 UTC (permalink / raw)
To: Jeff King
Cc: Dan McGee, GIT Mailing-list, Florian Achleitner,
David Michael Barr, Eric S. Raymond
In-Reply-To: <20130102065345.GA8685@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> [1] This symlink is doubly wrong, because any use of symbolic links
> in the test scripts needs to depend on the SYMLINKS prereq, and this
> does not.
Yeah, I think we have discussed this once already in
http://thread.gmane.org/gmane.comp.version-control.git/210688/focus=210714
> [2] In both the current code and what I showed above, the test scripts
> depend on things in contrib/. This is probably a bad idea in
> general, as the quality of what goes into contrib is not as closely
> watched (especially with respect to things like portability).
> Certainly I would not have known to look more carefully at a patch
> to contrib/svn-fe for breakage to the test suite.
As long as such tests are made skippable with appropriate
prerequisites, I do not think it is bad to have their tests in t/; I
would say it is rather better than having them in contrib/ and leave
it not run by anybody, which happened to some of the stuff in
contrib/ already.
> ... possibly with a feature to
> make it easier to run tests out of contrib).
Yes, that certainly is a workable alternative.
^ permalink raw reply
* Re: Makefile dependency from 'configure' to 'GIT-VERSION-FILE'
From: Jonathan Nieder @ 2013-01-02 7:21 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: Stefano Lattarini, Jeff King, git
In-Reply-To: <CANiSa6jt7_ixi7L6U9sfpV2mvT_7zgYV+m+sLiXjkDsFehAuwA@mail.gmail.com>
Hi Martin,
Martin von Zweigbergk wrote:
> I use autoconf with git.git. I have noticed lately, especially when
> doing things like "git rebase -i --exec make", that ./configure is run
> every time. If I understand correctly, this is because of 8242ff4
> (build: reconfigure automatically if configure.ac changes,
> 2012-07-19).
How about this patch (untested)?
-- >8 --
Subject: build: do not automatically reconfigure unless configure.ac changed
Starting with v1.7.12-rc0~4^2 (build: reconfigure automatically if
configure.ac changes, 2012-07-19), "config.status --recheck" is
automatically run every time the "configure" script changes. In
particular, that means the configuration procedure repeats whenever
the version number changes (since the configure script changes to
support "./configure --version" and "./configure --help"), making
bisecting painfully slow.
The intent was to make the reconfiguration process only trigger for
changes to configure.ac's logic. Tweak the Makefile rule to match
that intent by depending on configure.ac instead of configure.
Reported-by: Martin von Zweigbergk <martinvonz@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
[...]
> --- a/Makefile
> +++ b/Makefile
> @@ -2267,12 +2267,9 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
> mv $@+ $@
> endif # NO_PYTHON
>
> -configure: configure.ac GIT-VERSION-FILE
> +configure: configure.ac
[...]
> --- a/configure.ac
> +++ b/configure.ac
> @@ -142,7 +142,10 @@ fi
> ## Configure body starts here.
>
> AC_PREREQ(2.59)
> -AC_INIT([git], [@@GIT_VERSION@@], [git@vger.kernel.org])
> +AC_INIT([git],
> + m4_esyscmd([ ./GIT-VERSION-GEN &&
> + { sed -ne 's/GIT_VERSION = //p' GIT-VERSION-FILE | xargs echo -n; } ]),
> + [git@vger.kernel.org])
I don't think that would warrant dropping the GIT-VERSION-FILE
dependency, since the resulting configure script still hard-codes the
version number.
Sane?
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 736ecd45..2a22041f 100644
--- a/Makefile
+++ b/Makefile
@@ -2275,7 +2275,7 @@ configure: configure.ac GIT-VERSION-FILE
$(RM) $<+
ifdef AUTOCONFIGURED
-config.status: configure
+config.status: configure.ac
$(QUIET_GEN)if test -f config.status; then \
./config.status --recheck; \
else \
--
1.8.1
^ permalink raw reply related
* Re: [PATCH] gitk: add a checkbox to control the visibility of tags
From: Junio C Hamano @ 2013-01-02 7:24 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Łukasz Stelmach
In-Reply-To: <20130102071701.GG20724@iris.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> On Sat, Dec 01, 2012 at 06:16:25PM -0800, Junio C Hamano wrote:
>> Łukasz Stelmach <stlman@poczta.fm> writes:
>>
>> > Enable hiding of tags displayed in the tree as yellow labels.
>> > If a repository is used together with a system like Gerrit
>> > there may be quite a lot of tags used to control building
>> > and there may be hardly any place left for commit subjects.
>> >
>> > Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
>> > ---
>> ...
> If the concern is the amount of screen real-estate that the tags take
> up when there are many of them (which is a reasonable concern), I'd
> rather just put a single tag icon with "tags..." inside it and arrange
> to list all the tags in the diff display pane when the user clicks on
> it. I think that would be better than not showing the tags at all.
Yeah, sounds very sensible. Thanks.
Łukasz, what do you think?
^ permalink raw reply
* Re: Makefile dependency from 'configure' to 'GIT-VERSION-FILE'
From: Martin von Zweigbergk @ 2013-01-02 7:47 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Stefano Lattarini, Jeff King, git
In-Reply-To: <20130102072141.GB18974@elie.Belkin>
On Tue, Jan 1, 2013 at 11:21 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> How about this patch (untested)?
Looks good. Thanks!
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2267,12 +2267,9 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
>> mv $@+ $@
>> endif # NO_PYTHON
>>
>> -configure: configure.ac GIT-VERSION-FILE
>> +configure: configure.ac
> [...]
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -142,7 +142,10 @@ fi
>> ## Configure body starts here.
>>
>> AC_PREREQ(2.59)
>> -AC_INIT([git], [@@GIT_VERSION@@], [git@vger.kernel.org])
>> +AC_INIT([git],
>> + m4_esyscmd([ ./GIT-VERSION-GEN &&
>> + { sed -ne 's/GIT_VERSION = //p' GIT-VERSION-FILE | xargs echo -n; } ]),
>> + [git@vger.kernel.org])
>
> I don't think that would warrant dropping the GIT-VERSION-FILE
> dependency, since the resulting configure script still hard-codes the
> version number.
Yeah, you're right. I was merely sweeping the dependency under the rug :-(
>
> diff --git a/Makefile b/Makefile
> index 736ecd45..2a22041f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2275,7 +2275,7 @@ configure: configure.ac GIT-VERSION-FILE
> $(RM) $<+
>
> ifdef AUTOCONFIGURED
> -config.status: configure
> +config.status: configure.ac
> $(QUIET_GEN)if test -f config.status; then \
> ./config.status --recheck; \
> else \
The next line just outside the context here does depend on
'configure', which is why I thought this would not be right. But it
seems impossible to get away from that, and AUTOCONFIGURED should only
be set when ./configure has been run (IIUC), so it's not even
realistic to have "git reconfigure" fail to find "./configure". So,
again, looks good.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox