Re: Pack bitmaps on Git for Windows
On Tue, Apr 22, 2014 at 05:58:10PM +1000, Bryan Turner wrote: > It looks like the documentation for bitmaps is being included in the > 1.9.2 release of Git for Windows but the functionality itself is not > present. For example, doc\git\html\git-config.html includes settings > like these: > > pack.useBitmaps I think this is a documentation problem with Git for Windows. The bitmap code is going into v2.0, but there should be nothing (not even documentation) in v1.9.2. I don't know how the msysgit folks build their documentation, but I wonder if it accidentally built from current 'master' rather than v1.9.2. You might get more information at the msysgit mailing list: https://groups.google.com/forum/#!forum/msysgit -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 (Apr 2014, #07; Tue, 22)
> > * jx/blame-align-relative-time (2014-04-22) 2 commits > - blame: dynamic blame_date_width for different locales > - blame:: fix broken time_buf paddings in relative timestamp Should substitute the double colons (in blame::) with one. > > "git blame" miscounted number of columns needed to show localized > timestamps, resulting in jaggy left-side-edge of the source code > lines in its output. > > Will merge to 'next' and keep it there for the remainder of the cycle. > > -- Jiang Xin -- 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: [RFC] Speed up "git status" by caching untracked file info
On Wed, Apr 23, 2014 at 1:56 AM, Karsten Blees wrote:
> Am 22.04.2014 12:35, schrieb Duy Nguyen:
>> On Tue, Apr 22, 2014 at 5:13 PM, Duy Nguyen wrote:
IIRC name_hash.c::lazy_init_name_hash took ~100ms on my system, so
hopefully you did a dummy 'cache_name_exists("anything")' before starting
the measurement of the first run?
>>>
>>> No I didn't. Thanks for pointing it out. I'll see if I can reduce its time.
>>
>> Well name-hash is only used when core.ignorecase is set. So it's
>> optional.
>
> This is only true for the case-insensitive directory hash. The file hash
> ('cache_file_exists') is always used to skip expensive excluded checks for
> tracked files.
>
> 'cache_file_exists' basically treats faster lookups for higher setup costs,
> which makes perfect sense when scanning the entire work tree. However, if
> most of the directory info is cached and just a few directories need refresh
> (and core.ignorecase=false), binary search ('cache_name_pos') may be better.
> The difficulty is to decide when to choose one over the other :-)
Right. The problem is even if untracked cache is used, we don't know
in advance how cache_file_exists calls we need to make. If .gitignore
changes, we could see how many directories are invalidated recursively
and that could be an indicator for favoring cache_file_exists over
cache_name_pos. It's harder when dir mtime changes, I suppose we could
be optimistic and stick to cache_name_pos until the number of calls
gets over a limit and turn to cache_file_exists. May backfire though..
>
>> Maybe we could save it in a separate index extension, but we
>> need to verify that the reader uses the same hash function as the
>> writer.
>>
Similarly, the '--directory' option controls early returns from the
directory scan (via read_directory_recursive's check_only argument), so
you won't be able to get a full untracked files listing if the cache was
recorded with '--directory'. Additionally, '--directory' aggregates the
state at the topmost untracked directory, so that directory's cached state
depends on all sub-directories as well...
>>>
>>> I missed this. We could ignore check_only if caching is enabled, but
>>> that does not sound really good. Let me think about it more..
>>
>> We could save "check_only" to the cache as well. This way we don't
>> have to disable the check_only trick completely.
>>
>> So we process a directory with check_only set, find one untracked
>> entry and stop short. We store check_only value and the status ("found
>> something") in addition to dir mtime. Next time we check the dir's
>> mtime. If it matches and is called with check_only set, we know there
>> is at least one untracked entry, that's enough to stop r_d_r and
>> return early. If dir mtime does not match, or r_d_r is called without
>> check_only, we ignore the cached data and fall back to opendir.
>>
>> Sounds good?
>>
>
> What about untracked files in sub-directories? E.g. you have untracked dirs
> a/b with untracked file a/b/c, so normal 'git status' would list 'a/' as
> untracked.
> Now, 'rm a/b/c' would update mtime of b, but not of a, so you'd still list
> 'a/' as untracked. Same thing for 'echo "c" >a/b/.gitignore'.
>
> Your solution could work if you additionally cache the directories that had
> to be scanned to find that first untracked file (but you probably had that in
> mind anyway).
Basically all directories that are touched by r_d_r() will be cached.
> If the cache is only used for certain dir_struct.flags combinations, you can
> probably get around saving the check_only flag (which can only ever be true
> if both DIR_SHOW_OTHER_DIRECTORIES and DIR_HIDE_EMPTY_DIRECTORIES are set
> (which is the default for 'git status')).
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pack-bitmap: do not core dump
On Apr 22, 2014, at 16:17, Jeff King wrote: but I do not think that is necessarily any more readable, especially because we probably need to cast it like: self->rlw = (eword_t *)((uint8_t *)self->buffer + rlw_offset); I suspect that will produce a warning about a cast increasing pointer alignment in some cases. So why do any uint8_t math in the first place? Just guessing it started out as uint8_t math throughout then the warning about increasing alignment showed up so that part got changed but the save pointer offset part did not. I think we could write it as: eword_t *old = self->buffer; ... realloc ... self->rlw = self->buffer + (self->rlw - old); Yeah that seems good too. I'm fine with your patch, though. thanks. I tend to gravitate towards the most minimal change that fixes the issue when touching someone else's code especially if I'm not familiar with it. There's also the minor issue of optimizing out the pointer arithmetic implicit divide by sizeof(eword_t) for the subtract and the implicit multiply by sizeof(eword_t) for the add -- I didn't check to see if one of the alternatives is best -- the one you mention above with the cast (keeping the uint8_t math) is probably guaranteed not to have any implicit multiply, but there's that pesky warning to get rid of. Of course those multiplies and divides should just end up being shifts so not a big deal if they didn't get optimized out (the realloc time should dwarf them into irrelevancy anyway). --Kyle -- 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] git-rebase: fix probable reflog typo
Commit 26cd160 (rebase -i: use a better reflog message) tried to produce a better reflog message, however, it seems a statement was introduced by mistake. 'comment_for_reflog start' basically overides the GIT_REFLOG_ACTION we just set. Signed-off-by: Felipe Contreras --- git-rebase--interactive.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 43631b4..5f1d8c9 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -893,8 +893,6 @@ then GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" output git checkout "$switch_to" -- || die "Could not checkout $switch_to" - - comment_for_reflog start fi orig_head=$(git rev-parse --verify HEAD) || die "No HEAD?" -- 1.9.2+fc1.1.g5c924db -- 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] pack-bitmap: do not core dump
On Tue, Apr 22, 2014 at 03:53:02PM -0700, Kyle J. McKay wrote: > So I was trying to use pack.writebitmaps=true and all I got was core dumps. Eek. > The fix with a real subject line ;) is below. I think perhaps this should be > picked up for the 2.0.0 release. (Patch is against master.) Yes, this is definitely the sort of bugfix we want to see during the -rc period (well, we would prefer not to see bugs at all, but if we must have them, fixes are helpful). > >8 > Subject: [PATCH] ewah_bitmap.c: do not assume size_t and eword_t are the same > size Thanks for a very well-written commit message. I think your fix makes sense: > - self->rlw = self->buffer + (rlw_offset / sizeof(size_t)); > + self->rlw = self->buffer + (rlw_offset / sizeof(eword_t)); We could also write it as: self->rlw = (uint8_t *)self->buffer + rlw_offset; but I do not think that is necessarily any more readable, especially because we probably need to cast it like: self->rlw = (eword_t *)((uint8_t *)self->buffer + rlw_offset); Given that self->rlw is a pointer to eword_t, though, we can assume rlw_offset is always going to be a multiple of sizeof(eword_t) anyway (and if it is not, the division in the original is a big problem, but I do not think that is the case). So why do any uint8_t math in the first place? I think we could write it as: eword_t *old = self->buffer; ... realloc ... self->rlw = self->buffer + (self->rlw - old); I'm fine with your patch, though. I also poked through the rest of the bitmap code looking for similar problems, but didn't find any. I do not think this was a systemic issue with bad use of types; it was just a think-o that happened to work on 64-bit machines. -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
What's cooking in git.git (Apr 2014, #07; Tue, 22)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The tip of the 'master' branch has passed v2.0.0-rc0, an early preview release. There are a handful of topics still in 'next' (and a few that are not in 'next') that I'd like to have in v2.0.0-rc1, but other than that, this hopefully is fairly a close approximation of the upcoming release. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * fc/remote-helpers-hg-bzr-graduation (2014-04-21) 3 commits - remote-helpers: move tests out of contrib - remote-helpers: move out of contrib - remote-helpers: squelch python import exceptions Move remote-hg and remote-bzr out of contrib/. Will merge to 'next' and keep it there for the remainder of the cycle. * rs/ref-transaction (2014-04-22) 14 commits . SQUASH??? . refs.c: change update_ref to use a transaction . walker.c: use ref transaction for ref updates . branch.c: use ref transaction for all ref updates . fast-import.c: change update_branch to use ref transactions . sequencer.c: use ref transactions for all ref updates . commit.c: use ref transactions for updates . replace.c: use the ref transaction functions for updates . tag.c: use ref transactions when doing updates . refs.c: ref_transaction_delete to check for error and return status . refs.c: change ref_transaction_create to do error checking and return status . refs.c: change ref_transaction_update() to do error checking and return status . refs.c: use a single exit path from transaction commit and handle onerr . refs.c: constify the sha arguments for ref_transaction_create|delete|update (this branch uses mh/ref-transaction.) Updates most of the callsites to write_sha1_ref(), the low-level mechanism to update a ref, to use the ref-transaction API. Seems to break the dumb walker test (t5550) when merged to 'pu', possibly due to misconversion of walker.c. Kept out of 'pu' as I didn't want to resolve conflicts with the other topics only to get a known-broken version. * fc/remote-helper-refmap (2014-04-21) 8 commits - transport-helper: remove unnecessary strbuf resets - transport-helper: add support to delete branches - fast-export: add support to delete refs - fast-import: add support to delete refs - transport-helper: add support to push symbolic refs - transport-helper: add support for old:new refspec - fast-export: add new --refspec option - fast-export: improve argument parsing Allow remote-helper/fast-import based transport to rename the refs while transferring the history. Will merge to 'next' and keep it there for the remainder of the cycle. * jk/external-diff-use-argv-array (2014-04-21) 6 commits - run_external_diff: refactor cmdline setup logic - run_external_diff: hoist common bits out of conditional - run_external_diff: drop fflush(NULL) - run_external_diff: clean up error handling - run_external_diff: use an argv_array for the environment - run_external_diff: use an argv_array for the command line Code clean-up. Will keep in 'next' for the remainder of the cycle. * jx/blame-align-relative-time (2014-04-22) 2 commits - blame: dynamic blame_date_width for different locales - blame:: fix broken time_buf paddings in relative timestamp "git blame" miscounted number of columns needed to show localized timestamps, resulting in jaggy left-side-edge of the source code lines in its output. Will merge to 'next' and keep it there for the remainder of the cycle. * fc/merge-default-to-upstream (2014-04-22) 1 commit (merged to 'next' on 2014-04-22 at 4f98483) + merge: enable defaulttoupstream by default "git merge" without argument, even when there is an upstream defined for the current branch, refused to run until merge.defaultToUpstream is set to true. Flip the default of that configuration variable to true. Will keep in 'next' for the remainder of the cycle. * fc/mergetool-prompt (2014-04-22) 1 commit (merged to 'next' on 2014-04-22 at dcaec94) + mergetool: run prompt only if guessed tool mergetool.prompt used to default to 'true', always causing a confirmation "do you really want to run the tool on this path" to be shown. Among the two purposes the prompt serves, ignore the use case to confirm that the user wants to view particular path with the named tool, and make the prompt only to confirm the choice of the tool made by autodetection and defaulting. For those who configured the tool explicitly, the prompt shown for the latter purpose is simply annoying. Strictly speaking, this is a backward incompatible change and the users need to explicitly set the variable to 'true' if they want to resurrect the now-ignored use case. Will keep in 'next' for the remainder of the cycle.
Re: [PATCH] pack-bitmap: do not core dump
"Kyle J. McKay" writes: > So I was trying to use pack.writebitmaps=true and all I got was core dumps. > > The fix with a real subject line ;) is below. I think perhaps this should be > picked up for the 2.0.0 release. (Patch is against master.) Of course---a breakage in a new code may be less important than a regression fix in the sense that there is an option not to use the new feature, but an obvious fix to such a breakage is certainly very much welcomed during the -rc period. Thanks. I'll try not to forget until tomorrow's integration cycle (I just finished and pushed the results out for today), which will also give me a chance to wait for Peff's Ack ;-). > --Kyle > > >8 > Subject: [PATCH] ewah_bitmap.c: do not assume size_t and eword_t are the same > size > > When buffer_grow changes the size of the buffer using realloc, > it first computes and saves the rlw pointer's offset into the > buffer using (uint8_t *) math before the realloc but then > restores it using (eword_t *) math. > > In order to do this it's necessary to convert the (uint8_t *) > offset into an (eword_t *) offset. It was doing this by > dividing by the sizeof(size_t). Unfortunately sizeof(size_t) > is not same as sizeof(eword_t) on all platforms. > > This causes illegal memory accesses and other bad things to > happen when attempting to use bitmaps on those platforms. > > Fix this by dividing by the sizeof(eword_t) instead which > will always be correct for all platforms. > > Signed-off-by: Kyle J. McKay > --- > ewah/ewah_bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c > index 9ced2dad..fccb42b5 100644 > --- a/ewah/ewah_bitmap.c > +++ b/ewah/ewah_bitmap.c > @@ -41,7 +41,7 @@ static inline void buffer_grow(struct ewah_bitmap *self, > size_t new_size) > self->alloc_size = new_size; > self->buffer = ewah_realloc(self->buffer, > self->alloc_size * sizeof(eword_t)); > - self->rlw = self->buffer + (rlw_offset / sizeof(size_t)); > + self->rlw = self->buffer + (rlw_offset / sizeof(eword_t)); > } > > static inline void buffer_push(struct ewah_bitmap *self, eword_t value) -- 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] pack-bitmap: do not core dump
So I was trying to use pack.writebitmaps=true and all I got was core dumps. The fix with a real subject line ;) is below. I think perhaps this should be picked up for the 2.0.0 release. (Patch is against master.) --Kyle >8 Subject: [PATCH] ewah_bitmap.c: do not assume size_t and eword_t are the same size When buffer_grow changes the size of the buffer using realloc, it first computes and saves the rlw pointer's offset into the buffer using (uint8_t *) math before the realloc but then restores it using (eword_t *) math. In order to do this it's necessary to convert the (uint8_t *) offset into an (eword_t *) offset. It was doing this by dividing by the sizeof(size_t). Unfortunately sizeof(size_t) is not same as sizeof(eword_t) on all platforms. This causes illegal memory accesses and other bad things to happen when attempting to use bitmaps on those platforms. Fix this by dividing by the sizeof(eword_t) instead which will always be correct for all platforms. Signed-off-by: Kyle J. McKay --- ewah/ewah_bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ewah/ewah_bitmap.c b/ewah/ewah_bitmap.c index 9ced2dad..fccb42b5 100644 --- a/ewah/ewah_bitmap.c +++ b/ewah/ewah_bitmap.c @@ -41,7 +41,7 @@ static inline void buffer_grow(struct ewah_bitmap *self, size_t new_size) self->alloc_size = new_size; self->buffer = ewah_realloc(self->buffer, self->alloc_size * sizeof(eword_t)); - self->rlw = self->buffer + (rlw_offset / sizeof(size_t)); + self->rlw = self->buffer + (rlw_offset / sizeof(eword_t)); } static inline void buffer_push(struct ewah_bitmap *self, eword_t value) -- 1.8.5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] Git v2.0.0-rc0
"brian m. carlson" writes: > On Tue, Apr 22, 2014 at 03:11:48PM -0700, Jonathan Nieder wrote: >> Another possibility would be to require Perl 5.8.9 or newer. It was >> released in 2008. > > RHEL 5 and CentOS 5 are still shipping with 5.8.8. They are still > security-supported until 2017, and believe it or not people still > develop on them. I am personally fine with this change, though. > > What we could do instead is simply require a newer version of > Getopt::Long, which would let people continue using their ancient OSes > and install a newer version from CPAN if necessary. It's also the > proper way to specify the dependency. Yes, but if its inability to properly grok --option="" is the only reason we want to add a dependency, wouldn't it suffice to simply state in the documentation (1) how to recognise the symptom to see if the version the user has is too old, e.g. "if you see this error message", "run 'perl -v' to see if your perl is older than X", etc. and (2) how to work it around, i.e. "instead of giving an empty value with --option='', say --option ''"? -- 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 02/13] refs.c: use a single exit path from transaction commit and handle onerr
On Tue, Apr 22, 2014 at 12:11 PM, Junio C Hamano wrote:
> Ronnie Sahlberg writes:
>
>> diff --git a/refs.c b/refs.c
>> index 138ab70..9daf89e 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3414,12 +3414,12 @@ int ref_transaction_commit(struct ref_transaction
>> *transaction,
>> const char *msg, enum action_on_err onerr)
>> ...
>> + if (ret) {
>> + const char *str = "Cannot commit transaction.";
>> + switch (onerr) {
>> + case UPDATE_REFS_MSG_ON_ERR: error(str); break;
>> + case UPDATE_REFS_DIE_ON_ERR: die(str); break;
>> + case UPDATE_REFS_QUIET_ON_ERR: break;
>> + }
>> + }
>> return ret;
>> }
>
> I think this is a response to Michael's earlier review "do different
> callers want to give different error messages more suitable for the
> situation?". I suspect that the ideal endgame may end up all
> callers passing QUIET_ON_ERR and doing the error message themselves,
> e.g. branch.c::craete_branch() may end something like this:
>
> ...
> if (!dont_change_ref)
> if (ref_transaction_commit(..., UPDATE_REFS_QUIET_ON_ERR))
> die_errno(_("Failed to write branch '%s'"),
> skip_prefix(ref.buf, "refs/heads/));
>
> And in the meantime until the callers are converted, we may end up
> showing the fallback "Cannot commit transaction." but that would be
> OK during the development and polishing of this series.
>
That is a good idea.
I will try to address that in the next respin.
--
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 00/13] Use ref transactions from most callers
I will look at this once i finish the next respin. On Tue, Apr 22, 2014 at 12:34 PM, Junio C Hamano wrote: > Ronnie Sahlberg writes: > >> This patch series changes most of the places where the ref functions for >> locking and writing refs to instead use the new ref transaction API. There >> are still three more places where write_ref_sha1() is called from outside >> of refs.c but those all will require more complex work and review so those >> changes belong in a different patch series. >> >> Version 2: >> - Add a patch to ref_transaction_commit to make it honor onerr even if the >>error triggered in ref_Transaction_commit itself rather than in a call >>to other functions (that already honor onerr). >> - Add a patch to make the update_ref() helper function use transactions >>internally. >> - Change ref_transaction_update to die() instead of error() if we pass >>if a NULL old_sha1 but have have_old == true. >> - Change ref_transaction_create to die() instead of error() if new_sha1 >>is false but we pass it a null_sha1. >> - Change ref_transaction_delete die() instead of error() if we pass >>if a NULL old_sha1 but have have_old == true. >> - Change several places to do if(!transaction || ref_transaction_update() >>|| ref_Transaction_commit()) die(generic-message) instead of checking each >>step separately and having a different message for each failure. >>Most users are likely not interested in what step of the transaction >>failed and only whether it failed or not. >> - Change commit.c to only pass a pointer to ref_transaction_update >>iff current_head is non-NULL. >>The previous patch used to compute a garbage pointer for >>current_head->object.sha1 and relied on the fact that >> ref_transaction_update >>would not try to dereference this pointer if !!current_head was 0. >> - Updated commit message for the walker_fetch change to try to justify why >>the change in locking semantics should not be harmful. > > Will queue, but when applied on top of mh/ref-transaction and tested > standalone, it appears to fail test #14 of t5550-http-fetch-dumb.sh > for me. > > It seems that the culprit is that this step: > > git http-fetch -a -w heads/master-new $HEAD $(git config > remote.origin.url) && > > creates ".git/heads/master-new" when what it was asked to create was > ".git/refs/heads/master-new". Perhaps there is something fishy in > the conversion in walker.c? We used to do lock_ref_sha1() on > "heads/master-new", which prepended "refs/" prefix before calling > lock_ref_sha1_basic() that expects the full path from $GIT_DIR/, but > ref_transaction_update(), which wants to see the full path, is still > fed "heads/master-new" after this series. > -- 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 is missing from Git v2.0
On Tue, 22 Apr 2014, Theodore Ts'o wrote: On Tue, Apr 22, 2014 at 02:23:18PM -0500, Felipe Contreras wrote: I am not fundamentally opposed. I just do not think it would add much value to new people at this point, and it will actively hurt if we shoved barely cooked one in 2.0. You are probably biased in that you've used Git far much more than the average user has (or future new users). I think Junio has a really strong point. If the goal is to make life easier for new users, allowing them to save a few keystrokes is probably not the most significant thing we can do. And we have to balance this with the additional cognitive load in remembering how a particular two character alias maps to the "real" command. This is especially true for commands which might not be used as often -- e.g., "rebase", and for commands where the meaning of "git commit" without any argument is qualitatively different from what "ci" (for checkin) means in most other source management systems. So I do think it's worth thinking about this very carefully. For certain, I would **not** recommend using shortcuts in example command sequences. If the user reads "git rebase" or "git cherry-pick" it means a lot more than if they see a series of apparent chicken scratches filled with things like "git rb", "git pi", "git st", etc. In fact, to be fair, you may be getting biased because you're used to using the two character shortcuts, so for you, of *course* "rb" and "pi" and "ci" make a lot of sense. But for someone who is starting from scratch, I really question how much it helps, and how much it might hurt, to see the two character shortcuts or even to have to remember the two character shortcuts. And for a command like "rebase" where the user can very easily shoot themselves in the foot to begin with, I'd actually suggest that it's a _good_ thing that they have to type it out in full. agreed, of all the things that people complain about regarding learning git, the fact that the commands are words instead of cryptic 2 letter abberviations is not one of them. The complaints tend to be far more about how there are inconsistancies between commands, or they don't understand what's happening. Adding a new inconsistancy, or changing words to abbreviations is a further barrier against new users, not an advantage for them. David Lang -- 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: [ANNOUNCE] Git v2.0.0-rc0
On Tue, Apr 22, 2014 at 03:11:48PM -0700, Jonathan Nieder wrote: > Another possibility would be to require Perl 5.8.9 or newer. It was > released in 2008. RHEL 5 and CentOS 5 are still shipping with 5.8.8. They are still security-supported until 2017, and believe it or not people still develop on them. I am personally fine with this change, though. What we could do instead is simply require a newer version of Getopt::Long, which would let people continue using their ancient OSes and install a newer version from CPAN if necessary. It's also the proper way to specify the dependency. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: What is missing from Git v2.0
Felipe Contreras writes: > Theodore Ts'o wrote: > >> This is especially true for commands which might not be used as often >> -- e.g., "rebase", and for commands where the meaning of "git commit" >> without any argument is qualitatively different from what "ci" (for >> checkin) means in most other source management systems. > > ci means commit in Mercurial. Does it mean "commit the staging area"? >> In fact, to be fair, you may be getting biased because you're used to >> using the two character shortcuts, so for you, of *course* "rb" and >> "pi" and "ci" make a lot of sense. > > I can't be biased to those because I don't use them, mine are one > character shortcuts. Which you created yourself, on your own responsibility. > And if that hypothesis was correct, why does Mercurial, Bazaar, > Subversion, CVS, and pretty much everything uses aliases? And why does > pretty much every .gitconfig has similar aliases? That would imply > that the whole world is biased. Most .profile files define aliases as well. Doing this on the user's initiative is harmless since it will not have its usage leak into scripts intended for use by others. > It would help when the user starts to think "Geez, I seem to be typing > `git checkout` an awful lot, I wonder if there's a shortcut", which if > the .gitconfigs out there are any indication, it happens all the time. Actually, it happens very rarely if you are talking about _real_ gitconfigs deployed by projects as compared to _sample_ gitconfigs demonstrating Git features for _personal_ rather than project-wide use. >> And for a command like "rebase" where the user can very easily shoot >> themselves in the foot to begin with, I'd actually suggest that it's >> a _good_ thing that they have to type it out in full. > > And now you contradict yourself; you actually want to make life harder > for new users, on purpose. Life's hardness is not proportional to the number of typed characters or APL would be the easiest programming language narrowly followed by Perl. Life does not become easier by lowering input redundancy until it becomes hard to tell apart from line noise. Keyboards are highly efficient input devices, or we would not be conversing in whole sentences in technical mailing lists but rather in telegraphy style. -- David Kastrup -- 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.1] commit: add --ignore-submodules[=] parameter
On 18. 4. 2014 14:28, Jens Lehmann wrote:
> Am 13.04.2014 01:41, schrieb Ronald Weiss:
>> Second, there are some differences between adding standard ignored
>> files, and ignored submodules:
>>
>> 1) Already tracked files are never ignored, regardless of .gitignore.
>> However, tracked submodules should be ignored by "add -u", if told so
>> by their ignore setting.
>>
>> 2) So .gitignore seems to only do something when adding new files to
>> the repo. However, when adding new submodules, they are probably never
>> ignored (setting the ignore setting for non existent submodule seems
>> like non-sense, although possible).
>
> What about "diff.ignoreSubmodules=all"?
>
>> 3) Ignored files can be ignored less explicitely (in global gitignore,
>> or using a wildcard, or by ignoring parent folder). So it makes sense
>> to warn the user if he tries to explicitely add an ignored file, as he
>> might not be aware that the file is ignored. Submodules, however, can
>> only be ignored explicitely. And when user explicitely specifies the
>> submodule in an add command, he most probably really wants to add it,
>> so I don't see the point in warning him and requiring the -f option.
>
> But we do have "diff.ignoreSubmodules=all", so I do not agree with
> your conclusion.
Ah, I didn't know about diff.ignoreSubmodules. I agree that it defeats
two of my points :-(.
And how about the point 1), should submodules never be ignored once
already tracked, like standard ignored files? I'm almost sure that in
this case the behavior should be different, otherwise it would drop
the most useful use case of the proposed change.
>> So, I think that the use cases are completely different, for submodules
>> and ignored files. So trying to make add behave the same for both, might
>> not be that good idea.
>>
>> I would propose - let's make add honor the ignore setting by just
>> parsing if from config like the other commands do, and pass it to
>> underlying diff invocations. And at the same the, let's override it for
>> submodules explicitely specified on the command line, to never ignore
>> such submodules, without requiring the -f option. That seems to be
>> pretty easy, see below.
>
> What about doing that only when '-f' is given? Would that do what we
> want?
OK then, seems right with diff.ignoreSubmodules in mind.
But one question stays - what to do with submodules not explicitly
specified on command line (when their parent folder is being added)?
You adviced already to do what we do with standard ignored files. That
seems to be:
A) if the file is already tracked, add it
B) if not tracked, without -f, silently ignore it
C) if not tracked, with -f, add it
A) is same like point 1 above, let's forget it until we settle that one.
But B) is even more problematic, we would probably have to modify the
directory parsing code in dir.c to handle ignored submodules. Is that
what we want? If so, then I'm afraid this is getting overly complicated.
>> @@ -320,6 +324,7 @@ int cmd_add(int argc, const char **argv, const char
>> *prefix)
>> char *seen = NULL;
>>
>> git_config(add_config, NULL);
>> +gitmodules_config();
>
> Wrong order, gitmodules_config() must be called before git_config()
> to make the .gitmodules settings overridable by the users config.
Right. I noticed that too just after sending the patch.
>> @@ -440,6 +446,18 @@ int cmd_add(int argc, const char **argv, const char
>> *prefix)
>> die(_("pathspec '%s' did not match any
>> files"),
>> pathspec.items[i].original);
>> }
>> +
>> +/* disable ignore setting for any submodules specified
>> explicitly in the pathspec */
>> +if (path[0] &&
>> +(cachepos = cache_name_pos(path,
>> pathspec.items[i].len)) >= 0 &&
>> +S_ISGITLINK(active_cache[cachepos]->ce_mode)) {
>> +char *optname;
>> +int optnamelen = pathspec.items[i].len + 17;
>> +optname = xcalloc(optnamelen + 1, 1);
>> +snprintf(optname, optnamelen + 1,
>> "submodule.%s.ignore", path);
>> +parse_submodule_config_option(optname, "none");
>
> We should use "dirty" instead of "none" here. Modifications inside
> the submodule cannot be added without committing them there first
> and using "none" might incur an extra preformance penalty for
> looking through the submodule directories for such changes.
OK, that's right too.
--
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 is missing from Git v2.0
Matthieu Moy writes: > Felipe Contreras writes: > >> Why is not material for v2.0? Because you say so? Are you going to wait >> another >> ten years to introduce this to v3.0? > > There's no need to wait for a 3.0 to introduce this. If these would > be low-priority compared to user-defined aliases, there's no backward > compatibility issue, it can very well be a 2.1, or whatever number comes > after 2.0. I do not think the discussion has analysed the issue deeply enough for us to tell what the final proposal would look like, in order to judge what kind of issues, whether related to backward compatibility or not, are involved. My hunch is that this may not have to wait for a big version bump, but I am not sure about that at this point. Also I do not think 3.0 has to wait for ten years, either. We started discussing incompatible updates for 2.0 in earnest during the v1.8.2 timeframe, which was tagged mid-March 2013 with a release notes with a "B/c notes for 2.0" section, but IIRC the discussion of many of the "let's make things consistent (even if that means we have to break existing users), and devise ways to make the transtion for them as smooth as possible" changes that finally is going to happen in v2.0 have been in the works since the v1.7.x era (tagged late January 2012). While I do not want to rush things late in a cycle (notice how many topics are still cooking in 'next' and will continue to be in preparation for 2.1 right now), I do not think it is warranted to have a panic "3.0 will be ten years out, and we will miss the boat if we do not cram this in some shape into 2.0"---that kind of haste simply is not necessary. Even if we end up having to wait for 3.0, it will happen within two years max, if not earlier. -- 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: [ANNOUNCE] Git v2.0.0-rc0
Junio C Hamano wrote: > Jonathan Nieder writes: >> The documentation says >> >> --prefix= >> >> ... >> >> Before Git 2.0, the default prefix was "" (no prefix). >> This meant that ... >> >> which suggests that I can use --prefix="" to mean no prefix. Perhaps >> it needs a note to suggest using '--prefix ""' instead? > > Is there another --option that takes an arbitrary user string that > could be an empty string (or will there be one in the future)? In git in general, yes --- for example, 'git diff --src-prefix="" HEAD^' tells "git diff" to leave off the usual c/ prefix in the src filename it prints. In git-svn, --trunk="" or --message="" might be meaningful, but not nearly as much as --prefix="". > If > that is the case, a better alternative might be to add an comment to > say that those with older Getopt::Long may have to use --option "" > instead of the --option="" form for any option whose value happens > to be an empty string to work around the command parser bug. Another possibility would be to require Perl 5.8.9 or newer. It was released in 2008. diff --git i/git-svn.perl w/git-svn.perl index 0a32372..ec7910d 100755 --- i/git-svn.perl +++ w/git-svn.perl @@ -1,7 +1,7 @@ #!/usr/bin/perl # Copyright (C) 2006, Eric Wong # License: GPL v2 or later -use 5.008; +use 5.008_009; use warnings; use strict; use vars qw/ $AUTHOR $VERSION -- 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 is missing from Git v2.0
Theodore Ts'o wrote: > On Tue, Apr 22, 2014 at 02:23:18PM -0500, Felipe Contreras wrote: > > > I am not fundamentally opposed. I just do not think it would add > > > much value to new people at this point, and it will actively hurt > > > if we shoved barely cooked one in 2.0. > > > > You are probably biased in that you've used Git far much more than > > the average user has (or future new users). > > I think Junio has a really strong point. What is that point? > If the goal is to make life easier for new users, allowing them to save a few > keystrokes is probably not the most significant thing we can do. No, but it's *one* thing we can definetly do, and since we are not doing anything else, we might as well do this one thing. > And we have to balance this with the additional cognitive load in remembering > how a particular two character alias maps to the "real" command. What cognitive load? You don't *need* to remember the default aliases; you don't need to use them at all. > This is especially true for commands which might not be used as often -- > e.g., "rebase", and for commands where the meaning of "git commit" without > any argument is qualitatively different from what "ci" (for checkin) means in > most other source management systems. ci means commit in Mercurial. > So I do think it's worth thinking about this very carefully. For > certain, I would **not** recommend using shortcuts in example command > sequences. If the user reads "git rebase" or "git cherry-pick" it > means a lot more than if they see a series of apparent chicken > scratches filled with things like "git rb", "git pi", "git st", etc. Certainly, but that is orthogonal. > In fact, to be fair, you may be getting biased because you're used to > using the two character shortcuts, so for you, of *course* "rb" and > "pi" and "ci" make a lot of sense. I can't be biased to those because I don't use them, mine are one character shortcuts. And if that hypothesis was correct, why does Mercurial, Bazaar, Subversion, CVS, and pretty much everything uses aliases? And why does pretty much every .gitconfig has similar aliases? That would imply that the whole world is biased. > But for someone who is starting from scratch, I really question how much it > helps, and how much it might hurt, to see the two character shortcuts or even > to have to remember the two character shortcuts. Here you are talking from your own bias. It would help when the user starts to think "Geez, I seem to be typing `git checkout` an awful lot, I wonder if there's a shortcut", which if the .gitconfigs out there are any indication, it happens all the time. > And for a command like "rebase" where the user can very easily shoot > themselves in the foot to begin with, I'd actually suggest that it's a _good_ > thing that they have to type it out in full. And now you contradict yourself; you actually want to make life harder for new users, on purpose. -- Felipe Contreras -- 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 is missing from Git v2.0
Junio C Hamano writes: > I am not fundamentally opposed. I just do not think it would add > much value to new people at this point, and it will actively hurt > if we shoved barely cooked one in 2.0. > > A few thinking points that are necessary to be worked out, even > before we start imagining a plan to introduce them, off the top of > my head, are these: > > * Should we add many random aliases, or should we limit ourselves >only to a narrow set that everybody can agree on? > > * What is the additional cognitive load do we feel comfortable >burdening with the new users? > > * What is the exact mechanism to give these built-in aliases? > > Imagine that somebody says "[alias] ci = commit" and a handful of > vocal people on this list agree that it is a good idea. Many random > websites start mentioning "git ci -a" in their beginner examples. > > Users are led to think "ci" is a command just like "commit" and does > the same thing and need to learn that only the > first kind and not the second kind cannot be used to define their > own alias (and the users need to learn "commit" at that time as > well). A bit further on this point, looking into the future. It might help if we added some "introduction" phase to the first invocation of "git init", "git clone", etc. that is interactively run on a machine where there is no $HOME/.gitconfig detected, and help the user populate it with a few selected and uncontroversial ones. Such a session might go like this: $ git init ... We do not see $HOME/.gitconfig; using Git for the first time? ... Let us ask a few questions to make your Git life more pleasant. >> What name do you want to appear on your commits? Sebastian Schuberth >> What email address do you want to appear on your commits? sschube...@gmail.com >> You record your commits as ... "Sebastian Schuberth ". >> OK? Y >> Do you want us to add a few sample aliases to your configuration file? Y ... Done. You can further tweak $HOME/.gitconfig to suit ... Git to your taste. And it would end up with something like this: $ cat $HOME/.gitconfig [user] name = <> email = <> [alias] co = checkout lg = log --oneline which can serve as an example the user can then tweak, without giving any false impression that "co" is any more special than whatever the user adds as a custom alias. But the need to make the set minimum and the need to carefully think things through are still there (Ted made a very good point about "rb", which I agree with). -- 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: How to trim the fat on my log graphs
Robert Dailey writes: > git log log --graph --abbrev-commit --decorate --date=relative > --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold > green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim > white)%an%C(reset) - %C(white)%s%C(reset)' --branches --remotes > ... > The goal is to weed out the really distant and irrelevant commits. I > really just want to see the commits that are closely related to my > current branch. Thanks in advance. For a starter, how about dropping "--branches --remotes" from that command line? A merge from elsewhere will show as "Merge branch foo" which should be sufficient without the decoration. -- 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 is missing from Git v2.0
On Tue, Apr 22, 2014 at 02:23:18PM -0500, Felipe Contreras wrote: > > I am not fundamentally opposed. I just do not think it would add > > much value to new people at this point, and it will actively hurt > > if we shoved barely cooked one in 2.0. > > You are probably biased in that you've used Git far much more than > the average user has (or future new users). I think Junio has a really strong point. If the goal is to make life easier for new users, allowing them to save a few keystrokes is probably not the most significant thing we can do. And we have to balance this with the additional cognitive load in remembering how a particular two character alias maps to the "real" command. This is especially true for commands which might not be used as often -- e.g., "rebase", and for commands where the meaning of "git commit" without any argument is qualitatively different from what "ci" (for checkin) means in most other source management systems. So I do think it's worth thinking about this very carefully. For certain, I would **not** recommend using shortcuts in example command sequences. If the user reads "git rebase" or "git cherry-pick" it means a lot more than if they see a series of apparent chicken scratches filled with things like "git rb", "git pi", "git st", etc. In fact, to be fair, you may be getting biased because you're used to using the two character shortcuts, so for you, of *course* "rb" and "pi" and "ci" make a lot of sense. But for someone who is starting from scratch, I really question how much it helps, and how much it might hurt, to see the two character shortcuts or even to have to remember the two character shortcuts. And for a command like "rebase" where the user can very easily shoot themselves in the foot to begin with, I'd actually suggest that it's a _good_ thing that they have to type it out in full. - Ted -- 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: [ANNOUNCE] Git v2.0.0-rc0
Jonathan Nieder writes: > Junio C Hamano wrote: >> Jonathan Nieder writes: > >>> Hm, perhaps we should introduce a 'no-prefix' option to work around >>> this. > [...] >>> That way, normal usage of --prefix would still be consistent with >>> other git commands that prefer the form with argument attached >>> (--prefix=foo, not --prefix foo; see gitcli(7)). >>> >>> Thoughts? >> >> I do not think that it is a good idea to use "--no-anything" for >> something that is not a boolean. > > Do you mean it is a bad idea to support or a bad idea to make use of > such support? > > I suggested --no- for consistency with current git commands that use > parseopt. But on second thought, I agree that it be confusing for > > --prefix=foo --no-prefix > > to mean something different from no --prefix parameter at all. > > The documentation says > > --prefix= > > ... > > Before Git 2.0, the default prefix was "" (no prefix). > This meant that ... > > which suggests that I can use --prefix="" to mean no prefix. Perhaps > it needs a note to suggest using '--prefix ""' instead? Is there another --option that takes an arbitrary user string that could be an empty string (or will there be one in the future)? If that is the case, a better alternative might be to add an comment to say that those with older Getopt::Long may have to use --option "" instead of the --option="" form for any option whose value happens to be an empty string to work around the command parser bug. -- 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 v5 2/2] commit: add --ignore-submodules[=] parameter
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
Git commit honors the 'ignore' setting from .gitmodules or .git/config,
but didn't allow to override it from command line.
This patch depends on Jens Lehmann's patch "commit -m: commit staged
submodules regardless of ignore config". Without it,
"commit -m --ignore-submodules" would not work and tests introduced
here would fail.
Signed-off-by: Ronald Weiss
---
Changes against v4 of this patch:
* fixed file mode of added test script (644 -> 755)
* replaced test_might_fail with test_must_fail in test script
Documentation/git-commit.txt| 7
builtin/commit.c| 8 +++-
t/t7513-commit-ignore-submodules.sh | 78 +
3 files changed, 92 insertions(+), 1 deletion(-)
create mode 100755 t/t7513-commit-ignore-submodules.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0bbc8f5..de0e8fe 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -13,6 +13,7 @@ SYNOPSIS
[-F | -m ] [--reset-author] [--allow-empty]
[--allow-empty-message] [--no-verify] [-e] [--author=]
[--date=] [--cleanup=] [--[no-]status]
+ [--ignore-submodules[=]]
[-i | -o] [-S[]] [--] [...]
DESCRIPTION
@@ -277,6 +278,12 @@ The possible options are:
The default can be changed using the status.showUntrackedFiles
configuration variable documented in linkgit:git-config[1].
+--ignore-submodules[=]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none", "dirty, "untracked" or "all", which
+ is the default.
+
-v::
--verbose::
Show unified diff between the HEAD commit and what
diff --git a/builtin/commit.c b/builtin/commit.c
index a148e28..8c4d05e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -361,7 +361,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
*/
if (all || (also && pathspec.nr)) {
fd = hold_locked_index(&index_lock, 1);
- add_files_to_cache(also ? prefix : NULL, &pathspec, 0, NULL);
+ add_files_to_cache(also ? prefix : NULL, &pathspec, 0,
ignore_submodule_arg);
refresh_cache_or_die(refresh_flags);
update_main_cache_tree(WRITE_TREE_SILENT);
if (write_cache(fd, active_cache, active_nr) ||
@@ -1526,6 +1526,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass
post-rewrite hook")),
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
N_("mode"), N_("show untracked files, optional modes: all, normal, no.
(Default: all)"), PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+ { OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg,
N_("when"),
+ N_("ignore changes to submodules, optional when: all, none.
(Default: all)"),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
/* end commit contents options */
OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
@@ -1564,6 +1567,9 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_options,
builtin_commit_usage,
prefix, current_head, &s);
+
+ s.ignore_submodule_arg = ignore_submodule_arg;
+
if (dry_run)
return dry_run_commit(argc, argv, prefix, current_head, &s);
index_file = prepare_index(argc, argv, prefix, current_head, 0);
diff --git a/t/t7513-commit-ignore-submodules.sh
b/t/t7513-commit-ignore-submodules.sh
new file mode 100755
index 000..52ab37d
--- /dev/null
+++ b/t/t7513-commit-ignore-submodules.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Copyright (c) 2014 Ronald Weiss
+#
+
+test_description='Test of git commit --ignore-submodules'
+
+. ./test-lib.sh
+
+test_expect_success 'create submodule' '
+ test_create_repo sm && (
+ cd sm &&
+ >foo &&
+ git add foo &&
+ git commit -m "Add foo"
+ ) &&
+ git submodule add ./sm &&
+ git commit -m "Add sm"
+'
+
+update_sm () {
+ (cd sm &&
+ echo bar >> foo &&
+ git add foo &&
+ git commit -m "Updated foo"
+ )
+}
+
+test_expect_success 'commit -a --ignore-submodules=all ignores dirty
submodule' '
+ update_sm &&
+ test_must_fail git commit -a --ignore-submodules=all -m "Update sm"
+'
+
+test_expect_success 'commit -a --ignore-submodules=none overrides ignore=all
setting' '
+ update_sm &&
+ git config sub
[PATCH v5 1/2] add: add --ignore-submodules[=] parameter
Allow ignoring submodules (or not) by command line switch, like diff
and status do.
This commit is also a prerequisite for the next one in series, which
adds the --ignore-submodules switch to git commit. That's why a new
argument is added to public function add_files_to_cache(), and it's
call sites are updated to pass NULL.
Signed-off-by: Ronald Weiss
---
Git add currently doesn't honor ignore setting from .gitmodules or
.git/config, which is related functionality, however I'd like to
change that in another patch, coming soon.
Changes against v4 of this patch:
* fixed file mode of added test script (644 -> 755)
* cleaned up commit message
Documentation/git-add.txt| 7 ++-
builtin/add.c| 16 --
builtin/checkout.c | 2 +-
builtin/commit.c | 2 +-
cache.h | 2 +-
t/t3704-add-ignore-submodules.sh | 45
6 files changed, 68 insertions(+), 6 deletions(-)
create mode 100755 t/t3704-add-ignore-submodules.sh
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 9631526..b2c936f 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -11,7 +11,7 @@ SYNOPSIS
'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
[--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
[--intent-to-add | -N] [--refresh] [--ignore-errors]
[--ignore-missing]
- [--] [...]
+ [--ignore-submodules[=]] [--] [...]
DESCRIPTION
---
@@ -164,6 +164,11 @@ for "git add --no-all ...", i.e. ignored removed
files.
be ignored, no matter if they are already present in the work
tree or not.
+--ignore-submodules[=]::
+ Can be used to override any settings of the 'submodule.*.ignore'
+ option in linkgit:git-config[1] or linkgit:gitmodules[5].
+can be either "none" or "all", which is the default.
+
\--::
This option can be used to separate command-line options from
the list of files, (useful when filenames might be mistaken
diff --git a/builtin/add.c b/builtin/add.c
index 459208a..85f2110 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -83,7 +83,8 @@ static void update_callback(struct diff_queue_struct *q,
}
int add_files_to_cache(const char *prefix,
- const struct pathspec *pathspec, int flags)
+ const struct pathspec *pathspec, int flags,
+ const char *ignore_submodules_arg)
{
struct update_callback_data data;
struct rev_info rev;
@@ -99,6 +100,12 @@ int add_files_to_cache(const char *prefix,
rev.diffopt.format_callback = update_callback;
rev.diffopt.format_callback_data = &data;
rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
+
+ if (ignore_submodules_arg) {
+ DIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);
+ handle_ignore_submodules_arg(&rev.diffopt,
ignore_submodules_arg);
+ }
+
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
return !!data.add_errors;
}
@@ -237,6 +244,8 @@ static int ignore_add_errors, intent_to_add, ignore_missing;
static int addremove = ADDREMOVE_DEFAULT;
static int addremove_explicit = -1; /* unspecified */
+static char *ignore_submodule_arg;
+
static int ignore_removal_cb(const struct option *opt, const char *arg, int
unset)
{
/* if we are told to ignore, we are not adding removals */
@@ -262,6 +271,9 @@ static struct option builtin_add_options[] = {
OPT_BOOL( 0 , "refresh", &refresh_only, N_("don't add, only refresh the
index")),
OPT_BOOL( 0 , "ignore-errors", &ignore_add_errors, N_("just skip files
which cannot be added because of errors")),
OPT_BOOL( 0 , "ignore-missing", &ignore_missing, N_("check if - even
missing - files are ignored in dry run")),
+ { OPTION_STRING, 0, "ignore-submodules", &ignore_submodule_arg,
N_("when"),
+ N_("ignore changes to submodules, optional when: all, none. (Default:
all)"),
+ PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
OPT_END(),
};
@@ -434,7 +446,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
plug_bulk_checkin();
- exit_status |= add_files_to_cache(prefix, &pathspec, flags);
+ exit_status |= add_files_to_cache(prefix, &pathspec, flags,
ignore_submodule_arg);
if (add_new_files)
exit_status |= add_files(&dir, flags);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 07cf555..607af47 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -525,7 +525,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
* entries in the index.
*/
- add_files_to_cache(NULL, NULL, 0);
+ add_files_to_cache(NULL, NULL, 0, NULL);
Re: What is missing from Git v2.0
Matthieu Moy wrote: > Felipe Contreras writes: > > > Why is not material for v2.0? Because you say so? Are you going to wait > > another > > ten years to introduce this to v3.0? > > There's no need to wait for a 3.0 to introduce this. If these would > be low-priority compared to user-defined aliases, there's no backward > compatibility issue, it can very well be a 2.1, or whatever number comes > after 2.0. Can it? I would like Junio to say so. Still, v2.0 is a better time for this. > > This is actually the perfect time to do it. > > Junio has just tagged a -rc for 2.0, so it's clearly too late to start > discussing new features for this particular release. And whose fault is that? I never saw any warning about the -rc tag, and I never saw any discussion about possible features missing in v2.0. Given that the next backward-incompatible release might be in a decade, I would have expected people to have put more thought into v2.0, but I guess the Git project is not really interested in progressive features. -- Felipe Contreras -- 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: [ANNOUNCE] Git v2.0.0-rc0
Junio C Hamano wrote: > Jonathan Nieder writes: >> Hm, perhaps we should introduce a 'no-prefix' option to work around >> this. [...] >> That way, normal usage of --prefix would still be consistent with >> other git commands that prefer the form with argument attached >> (--prefix=foo, not --prefix foo; see gitcli(7)). >> >> Thoughts? > > I do not think that it is a good idea to use "--no-anything" for > something that is not a boolean. Do you mean it is a bad idea to support or a bad idea to make use of such support? I suggested --no- for consistency with current git commands that use parseopt. But on second thought, I agree that it be confusing for --prefix=foo --no-prefix to mean something different from no --prefix parameter at all. The documentation says --prefix= ... Before Git 2.0, the default prefix was "" (no prefix). This meant that ... which suggests that I can use --prefix="" to mean no prefix. Perhaps it needs a note to suggest using '--prefix ""' instead? 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
How to trim the fat on my log graphs
My log graphs are pretty insane sometimes because we converted our repo from SVN and haven't had a chance to delete all of the remote branches. We still have quite a few (maybe 20). When I do `git log`, I am shown about 10-15 vertical lines and the branch I currently have checked out isn't even at the top of the graph, it's burried somewhere one or two pages down. I think this is really confusing and makes my log virtually useless. Here is my log command: git log log --graph --abbrev-commit --decorate --date=relative --format=format:'%C(bold blue)%h%C(reset)%x09%C(bold green)(%ar)%C(reset)%C(bold yellow)%d%C(reset) %C(dim white)%an%C(reset) - %C(white)%s%C(reset)' --branches --remotes I realize the `git log` documentation has a whole section on trimming history so it is more useful, but after reading that section I am left completely confused and I have no idea which additional options I can add to trim the fat on my logs. Ideally what I want, for starters: - The branch I'm on should be at the top of the graph (first commit in the log graph should be the tip of my checked out branch). - Commits in other branches that are not ancestors of the current branch should not be shown. - Merges *into* my branch from other branches should show the graph line for that branch but allow me to specify a "depth", meaning for example that ancestors that are 3 "parents" away (great-grandchildren?) will not be rendered (the 3 is an example, I could specify any number) The goal is to weed out the really distant and irrelevant commits. I really just want to see the commits that are closely related to my current branch. Thanks in advance. -- 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 is missing from Git v2.0
Felipe Contreras writes: > Why is not material for v2.0? Because you say so? Are you going to wait > another > ten years to introduce this to v3.0? There's no need to wait for a 3.0 to introduce this. If these would be low-priority compared to user-defined aliases, there's no backward compatibility issue, it can very well be a 2.1, or whatever number comes after 2.0. > This is actually the perfect time to do it. Junio has just tagged a -rc for 2.0, so it's clearly too late to start discussing new features for this particular release. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [ANNOUNCE] Git v2.0.0-rc0
Jonathan Nieder writes:
> Hm, perhaps we should introduce a 'no-prefix' option to work around
> this.
> ...
>> |diff --git a/git-svn.perl b/git-svn.perl
>> |index 7349ffea..284f458a 100755
>> |--- a/git-svn.perl
>> |+++ b/git-svn.perl
>> |@@ -149,7 +149,7 @@ my ($_trunk, @_tags, @_branches, $_stdlayout);
>> my %icv;
>> my %init_opts = ( 'template=s' => \$_template, 'shared:s' => \$_shared,
>> 'trunk|T=s' => \$_trunk, 'tags|t=s@' => \@_tags,
>> 'branches|b=s@' => \@_branches, 'prefix=s' => \$_prefix,
> + 'no-prefix' => sub { $_prefix = "" },
>
>> 'stdlayout|s' => \$_stdlayout,
>> 'minimize-url|m!' => \$Git::SVN::_minimize_url,
>>'no-metadata' => sub { $icv{noMetadata} = 1 },
>
> That way, normal usage of --prefix would still be consistent with
> other git commands that prefer the form with argument attached
> (--prefix=foo, not --prefix foo; see gitcli(7)).
>
> Thoughts?
I do not think that it is a good idea to use "--no-anything" for
something that is not a boolean.
I can buy "--old-default-prefix", or "--empty-prefix", but running
"git svn --prefix ''" (or "--prefix=''") would be OK and logically
consistent anyway (i.e. the option tells us what string to add after
"refs/remotes/", and the old default that everybody hated were to
use an empty string), so...
--
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: [ANNOUNCE] Git v2.0.0-rc0
"Kyle J. McKay" writes:
> Alternatively this change can be made in git-svn.perl:
> |diff --git a/git-svn.perl b/git-svn.perl
> |index 7349ffea..284f458a 100755
> |--- a/git-svn.perl
> |+++ b/git-svn.perl
> |@@ -149,7 +149,7 @@ my ($_trunk, @_tags, @_branches, $_stdlayout);
> my %icv;
> my %init_opts = ( 'template=s' => \$_template, 'shared:s' => \$_shared,
> 'trunk|T=s' => \$_trunk, 'tags|t=s@' => \@_tags,
> - 'branches|b=s@' => \@_branches, 'prefix=s' => \$_prefix,
> + 'branches|b=s@' => \@_branches, 'prefix:s' => \$_prefix,
> 'stdlayout|s' => \$_stdlayout,
> 'minimize-url|m!' => \$Git::SVN::_minimize_url,
>'no-metadata' => sub { $icv{noMetadata} = 1 },
>
> Which makes the argument to --prefix optional (in which case it is set
> to "").
We do want to learn what prefix the user wants to use when they
start their command line with "git svn --prefix". Stopping the
command line right there and expecting it to default to whatever
built-in prefix is simply strange; the user could have omitted that
"--prefix" that lacks the argument. If the user did want us to use
the default by passing an empty string as its argument, both forms,
i.e. "--prefix ''" and "--prefix=''", ought to be usable, but if
older Getopt::Long() does not allow the latter form, at least the
former is the right way for the user to tell us that.
So I think your fix (workaround for a bug in older Getopt::Long) in
the patch is the right one. Johan may want to help me by pointing
out if I am missing something.
Thanks.
> I'm not really sure what the best thing to do here is. I thought the
> documentation talked somewhere about using --prefix="" (or just --prefix=)
> to get the old behavior, but now I can't find that. In that
> case I think probably just changing the tests to use --prefix ""
> instead of --prefix="" should take care of it especially since the log
> message for fe191fca (which actually changes the default) talks about
> using --prefix "" and not --prefix="". I have attached a patch below
> (against master) to make that change to the two affected subtests of t9117.
>
> --Kyle
>
> >8
> Subject: [PATCH] t9117: use --prefix "" instead of --prefix=""
>
> Versions of Perl's Getopt::Long module before 2.37 do not contain
> this fix that first appeared in Getopt::Long version 2.37:
>
> * Bugfix: With gnu_compat, --foo= will no longer trigger "Option
> requires an argument" but return the empty string.
>
> Instead of using --prefix="" use --prefix "" when testing an
> explictly empty prefix string in order to work with older versions
> of Perl's Getopt::Long module.
>
> Signed-off-by: Kyle J. McKay
> ---
> t/t9117-git-svn-init-clone.sh | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
> index dfed76fe..a66f43c6 100755
> --- a/t/t9117-git-svn-init-clone.sh
> +++ b/t/t9117-git-svn-init-clone.sh
> @@ -101,18 +101,18 @@ test_expect_success 'clone with -s/-T/-b/-t assumes
> --prefix=origin/' '
> rm -f warning
> '
>
> -test_expect_success 'init with -s/-T/-b/-t and --prefix="" still works' '
> +test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
> test ! -d project &&
> - git svn init -s "$svnrepo"/project project --prefix="" 2>warning &&
> + git svn init -s "$svnrepo"/project project --prefix "" 2>warning &&
> test_must_fail grep -q prefix warning &&
> test_svn_configured_prefix "" &&
> rm -rf project &&
> rm -f warning
> '
>
> -test_expect_success 'clone with -s/-T/-b/-t and --prefix="" still works' '
> +test_expect_success 'clone with -s/-T/-b/-t and --prefix "" still works' '
> test ! -d project &&
> - git svn clone -s "$svnrepo"/project --prefix="" 2>warning &&
> + git svn clone -s "$svnrepo"/project --prefix "" 2>warning &&
> test_must_fail grep -q prefix warning &&
> test_svn_configured_prefix "" &&
> rm -rf project &&
--
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 2/2] mergetool: run prompt only if guessed tool
Junio C Hamano writes: > Do you have reaction to the other one "[PATCH 1/2] merge: enable > defaulttoupstream by default"? Sorry, that other one is not even about difftool/mergetool. I just checked that patch myself, and found it sensible for the longer term. -- 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 1/2] merge: enable defaulttoupstream by default
Felipe Contreras writes:
> There's no point in this:
>
> % git merge
> fatal: No commit specified and merge.defaultToUpstream not set.
>
> We know the most likely scenario is that the user wants to merge the
> upstream, and if not, he can set merge.defaultToUpstream to false.
And a new possible failure case is when there is no upstream defined
for the current branch, which gets perfectly good new error message:
$ git merge
fatal: No remote for the current branch.
So I think this is good. We want to protect this with a new test,
no?
Will queue as-is for now.
> Signed-off-by: Felipe Contreras
> ---
> Documentation/git-merge.txt | 5 ++---
> builtin/merge.c | 2 +-
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index a3c1fa3..cf2c374 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -101,9 +101,8 @@ commit or stash your changes before running 'git merge'.
> Specifying more than one commit will create a merge with
> more than two parents (affectionately called an Octopus merge).
> +
> -If no commit is given from the command line, and if `merge.defaultToUpstream`
> -configuration variable is set, merge the remote-tracking branches
> -that the current branch is configured to use as its upstream.
> +If no commit is given from the command line, merge the remote-tracking
> +branches that the current branch is configured to use as its upstream.
> See also the configuration section of this manual page.
>
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 66d8843..1fc9319 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -63,7 +63,7 @@ static int verbosity;
> static int allow_rerere_auto;
> static int abort_current_merge;
> static int show_progress = -1;
> -static int default_to_upstream;
> +static int default_to_upstream = 1;
> static const char *sign_commit;
>
> static struct strategy all_strategy[] = {
--
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: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1
Richard Hansen writes: >> and plan for transition to forbid them >> everywhere in a next big version bump (it is too late for 2.0). > > Would it be acceptable to have a config option to forbid these in a > non-major version bump? Of course ;-) Because we try very hard to avoid a "flag day" change, any "plan for transition" inevitably has to include what we need to do _before_ the big version bump. > If it's OK to have a config option, then here's one possible transition > path (probably flawed, but my intent is to bootstrap discussion): > > 1. Add an option to forbid dangerous characters. The option defaults > to disabled for compatibility. If the option is unset, print a > warning upon encountering a ref name that would be forbidden. > 2. Later, flip the default to enabled. > 3. Later, in the weeks/months leading up to the next major version > release, print the warning even if the config option is set to > disabled. Sounds fairly conservative and nice. We may want to treat creating a new such ref and using an existing such ref differently, though, and that might give us a better/smoother transition (as you are, I am just thinking aloud). For example, it might be sufficient to do these two things: (1) upon an attempt to use an existing such ref, warn and encourage renaming of the ref. (2) upon an attempt to create a new one, error it out. in the first step, and in either case, tell the user about the loosening variable. Going that route may shorten the time until the initial safety. -- 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.c: treat RUN_SETUP_GENTLY and RUN_SETUP as mutually exclusive
"Luis R. Rodriguez" writes:
> From: "Luis R. Rodriguez"
>
> This saves us a few branches when RUN_SETUP is set up.
>
> Signed-off-by: Luis R. Rodriguez
> ---
Makes sense, especially because there is no sane reason to set both
bits on.
> git.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git.c b/git.c
> index 9efd1a3..7780572 100644
> --- a/git.c
> +++ b/git.c
> @@ -290,7 +290,7 @@ static int run_builtin(struct cmd_struct *p, int argc,
> const char **argv)
> if (!help) {
> if (p->option & RUN_SETUP)
> prefix = setup_git_directory();
> - if (p->option & RUN_SETUP_GENTLY) {
> + else if (p->option & RUN_SETUP_GENTLY) {
> int nongit_ok;
> prefix = setup_git_directory_gently(&nongit_ok);
> }
--
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 00/13] Use ref transactions from most callers
Ronnie Sahlberg writes: > This patch series changes most of the places where the ref functions for > locking and writing refs to instead use the new ref transaction API. There > are still three more places where write_ref_sha1() is called from outside > of refs.c but those all will require more complex work and review so those > changes belong in a different patch series. > > Version 2: > - Add a patch to ref_transaction_commit to make it honor onerr even if the >error triggered in ref_Transaction_commit itself rather than in a call >to other functions (that already honor onerr). > - Add a patch to make the update_ref() helper function use transactions >internally. > - Change ref_transaction_update to die() instead of error() if we pass >if a NULL old_sha1 but have have_old == true. > - Change ref_transaction_create to die() instead of error() if new_sha1 >is false but we pass it a null_sha1. > - Change ref_transaction_delete die() instead of error() if we pass >if a NULL old_sha1 but have have_old == true. > - Change several places to do if(!transaction || ref_transaction_update() >|| ref_Transaction_commit()) die(generic-message) instead of checking each >step separately and having a different message for each failure. >Most users are likely not interested in what step of the transaction >failed and only whether it failed or not. > - Change commit.c to only pass a pointer to ref_transaction_update >iff current_head is non-NULL. >The previous patch used to compute a garbage pointer for >current_head->object.sha1 and relied on the fact that > ref_transaction_update >would not try to dereference this pointer if !!current_head was 0. > - Updated commit message for the walker_fetch change to try to justify why >the change in locking semantics should not be harmful. Will queue, but when applied on top of mh/ref-transaction and tested standalone, it appears to fail test #14 of t5550-http-fetch-dumb.sh for me. It seems that the culprit is that this step: git http-fetch -a -w heads/master-new $HEAD $(git config remote.origin.url) && creates ".git/heads/master-new" when what it was asked to create was ".git/refs/heads/master-new". Perhaps there is something fishy in the conversion in walker.c? We used to do lock_ref_sha1() on "heads/master-new", which prepended "refs/" prefix before calling lock_ref_sha1_basic() that expects the full path from $GIT_DIR/, but ref_transaction_update(), which wants to see the full path, is still fed "heads/master-new" after this series. -- 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 is missing from Git v2.0
Junio C Hamano wrote: > Sebastian Schuberth writes: > > > On Mon, Apr 21, 2014 at 9:39 PM, Junio C Hamano wrote: > > > >>> If we don't standardize this now people will come up with their own > >>> definitions [1] [2] (and many others if you just search GitHub) which > >>> are again likely to differ (slightly), hindering interoperability. > >> > >> I am afraid that that ship has sailed long time ago, though. > > > > That's most likely true, but it does not get better by postponing this > > even more. > > As I already said: > > I think it might be OK to implement them as the lowest priority > fallback alias, so that '[alias] co = "user's definition"' > anywhere in the various configuration locations will override > it. I am a bit hesitant about adding start-up overhead, though. > Also I am not sure if people can agree with (1) a broadly wide > selection of aliases and (2) the actual definitions for them (I > am OK with "co === checkout" myself, but I'd rather not to even > think about my Git wasting cycles parsing extra configuration > items to support "br === branch" at all, for example). > > I am not fundamentally opposed. I just do not think it would add > much value to new people at this point, and it will actively hurt > if we shoved barely cooked one in 2.0. You are probably biased in that you've used Git far much more than the average user has (or future new users). > So even if we agree that it would be a good idea to have some > default fallback aliases, the set of such aliases we ship must be > limited to a set that everybody can agree on, both in the sense that > "adding alias XX is good" and also in the sense that "alias XX must > be defined as YY". > > As you allueded to, the Git userbase is a lot larger than it used to > be back in 2006, one alias, e.g. "[alias] br = branch", that is > reported as either useless or needed to be further tweaked by a > person on this list would mean that we would be either spending > unnecessary start-up cycles (for "useless" case) or adding cognitive > load of having to differente between "branch" and "br" (for "needs > further tweak" case) for thousands of users who would be better off > if we didn't have that specific alias. I think it's reasonable to follow these guidelines: 1) Each default alias should have two characters 2) Each default alias should map to a command without arguments 3) Each default alias be widely used in the wild This set matches the above: co = checkout ci = commit rb = rebase st = status br = branch dt = difftool mt = mergetool You might not like 'br', but there's tons of people already using that alias, so it's not "useless". I can go find links to many examples if you would like. > So while I understand the desire to have a bit more handholding and > am not fundamentally opposed to the desire, I am not optimistic that > an attempt to implement these "aliases" would result in a very > useful addition to the system, even if done after careful thought. The fact that you are not optimistic about it doesn't mean it's impossible. > In any case, this definitely is not a 2.0 material. I agree that it > would be good to start discussing it early (rather than later) if we > ever want to do such a change. Why is not material for v2.0? Because you say so? Are you going to wait another ten years to introduce this to v3.0? This is actually the perfect time to do it. -- Felipe Contreras -- 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 02/13] refs.c: use a single exit path from transaction commit and handle onerr
Ronnie Sahlberg writes:
> @@ -3481,6 +3481,14 @@ cleanup:
> unlock_ref(updates[i]->lock);
> free(delnames);
> ref_transaction_free(transaction);
> + if (ret) {
> + const char *str = "Cannot commit transaction.";
> + switch (onerr) {
> + case UPDATE_REFS_MSG_ON_ERR: error(str); break;
> + case UPDATE_REFS_DIE_ON_ERR: die(str); break;
> + case UPDATE_REFS_QUIET_ON_ERR: break;
> + }
> + }
> return ret;
> }
Also on top of this part:
- avoid complier warning for printf-like functions getting a non
literal format string as their format argument;
- style: case label and each statement on its own line.
- Allow localizing the error message.
diff --git a/refs.c b/refs.c
index e52b6bf..35ce61a 100644
--- a/refs.c
+++ b/refs.c
@@ -3515,11 +3515,16 @@ cleanup:
free(delnames);
ref_transaction_free(transaction);
if (ret) {
- const char *str = "Cannot commit transaction.";
+ const char *str = _("Cannot commit transaction.");
switch (onerr) {
- case UPDATE_REFS_MSG_ON_ERR: error(str); break;
- case UPDATE_REFS_DIE_ON_ERR: die(str); break;
- case UPDATE_REFS_QUIET_ON_ERR: break;
+ case UPDATE_REFS_MSG_ON_ERR:
+ error("%s", str);
+ break;
+ case UPDATE_REFS_DIE_ON_ERR:
+ die("%s", str);
+ break;
+ case UPDATE_REFS_QUIET_ON_ERR:
+ break;
}
}
return ret;
--
2.0.0-rc0-187-g5842ffa
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] commit: add --ignore-submodules[=] parameter
Am 22.04.2014 00:08, schrieb Ronald Weiss: > On 18. 4. 2014 14:09, Jens Lehmann wrote: >> Am 13.04.2014 00:49, schrieb Ronald Weiss: >>> Allow ignoring submodules (or not) by command line switch, like diff >>> and status do. >>> >>> Git commit honors the 'ignore' setting from .gitmodules or .git/config, >>> but didn't allow to override it from command line. >>> >>> This patch depends on Jens Lehmann's patch "commit -m: commit staged >>> submodules regardless of ignore config". Without it, >>> "commit -m --ignore-submodules" would not work and tests introduced >>> here would fail. >> >> Apart from the flags of the test (see below) I get three failures when >> running t7513. And I'm wondering why you are using "test_might_fail" >> there, shouldn't we know exactly if a commit should succeed or not >> and test exactly for that? > > I used "test_might_fail" instead of "test_must_fail" to denote that this > test doesn't care whether "git commit" fails or not due to empty commit > message. I found it more appropriate. However, if you disagree, I can > change it to "test_must_fail", no problem for me. I'd rather have them fail because nothing is to be committed (and not because the commit message is empty) and document we expect that to happen by using test_must_fail (and that for example will catch a later change which accidentally makes commit create empty commits here). > Unfortunately I don't know why the test fails for you, they pass for me. > Did you apply it on top of your own patch for "commit -m", which is > a prerequisite? Silly me, I forgot to do that (even though you explicitly mention this dependency in the commit message). Sorry for the noise, all tests run fine after rebasing your changes on top of mine. > I tried it again, on top of current master (cc29195 [tag: v2.0.0-rc0]). > First, I have applied your patch from here: > > http://article.gmane.org/gmane.comp.version-control.git/245783 > > On top of that, I applied my two patches, and the tests pass fine here. > Until now I was testing on Windows, but now I tested on a Linux box, > and that makes no difference. > -- 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 02/13] refs.c: use a single exit path from transaction commit and handle onerr
Ronnie Sahlberg writes:
> diff --git a/refs.c b/refs.c
> index 138ab70..9daf89e 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3414,12 +3414,12 @@ int ref_transaction_commit(struct ref_transaction
> *transaction,
> const char *msg, enum action_on_err onerr)
> ...
> + if (ret) {
> + const char *str = "Cannot commit transaction.";
> + switch (onerr) {
> + case UPDATE_REFS_MSG_ON_ERR: error(str); break;
> + case UPDATE_REFS_DIE_ON_ERR: die(str); break;
> + case UPDATE_REFS_QUIET_ON_ERR: break;
> + }
> + }
> return ret;
> }
I think this is a response to Michael's earlier review "do different
callers want to give different error messages more suitable for the
situation?". I suspect that the ideal endgame may end up all
callers passing QUIET_ON_ERR and doing the error message themselves,
e.g. branch.c::craete_branch() may end something like this:
...
if (!dont_change_ref)
if (ref_transaction_commit(..., UPDATE_REFS_QUIET_ON_ERR))
die_errno(_("Failed to write branch '%s'"),
skip_prefix(ref.buf, "refs/heads/));
And in the meantime until the callers are converted, we may end up
showing the fallback "Cannot commit transaction." but that would be
OK during the development and polishing of this series.
--
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: [ANNOUNCE] Git v2.0.0-rc0
Kyle J. McKay wrote:
> The problem with --prefix="" is this (from the Getopt::Long CHANGES file):
>
> Changes in version 2.37
> ---
>
> * Bugfix: With gnu_compat, --foo= will no longer trigger "Option
>requires an argument" but return the empty string.
>
> The system I ran the tests on happens to have Getopt::Long version 2.35.
Thanks for catching it.
Getopt::Long was updated to 2.37 in perl 5.8.9 (in 5.8.8 it was
updated to 2.35). INSTALL still only recommends Perl 5.8 so that's an
issue.
> The --prefix="" option can be rewritten --prefix "" in both tests and then
> they pass.
Hm, perhaps we should introduce a 'no-prefix' option to work around
this.
> |diff --git a/git-svn.perl b/git-svn.perl
> |index 7349ffea..284f458a 100755
> |--- a/git-svn.perl
> |+++ b/git-svn.perl
> |@@ -149,7 +149,7 @@ my ($_trunk, @_tags, @_branches, $_stdlayout);
> my %icv;
> my %init_opts = ( 'template=s' => \$_template, 'shared:s' => \$_shared,
> 'trunk|T=s' => \$_trunk, 'tags|t=s@' => \@_tags,
> 'branches|b=s@' => \@_branches, 'prefix=s' => \$_prefix,
+ 'no-prefix' => sub { $_prefix = "" },
> 'stdlayout|s' => \$_stdlayout,
> 'minimize-url|m!' => \$Git::SVN::_minimize_url,
>'no-metadata' => sub { $icv{noMetadata} = 1 },
That way, normal usage of --prefix would still be consistent with
other git commands that prefer the form with argument attached
(--prefix=foo, not --prefix foo; see gitcli(7)).
Thoughts?
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: [RFC] Speed up "git status" by caching untracked file info
Am 22.04.2014 12:35, schrieb Duy Nguyen:
> On Tue, Apr 22, 2014 at 5:13 PM, Duy Nguyen wrote:
>>> IIRC name_hash.c::lazy_init_name_hash took ~100ms on my system, so
>>> hopefully you did a dummy 'cache_name_exists("anything")' before starting
>>> the measurement of the first run?
>>
>> No I didn't. Thanks for pointing it out. I'll see if I can reduce its time.
>
> Well name-hash is only used when core.ignorecase is set. So it's
> optional.
This is only true for the case-insensitive directory hash. The file hash
('cache_file_exists') is always used to skip expensive excluded checks for
tracked files.
'cache_file_exists' basically treats faster lookups for higher setup costs,
which makes perfect sense when scanning the entire work tree. However, if most
of the directory info is cached and just a few directories need refresh (and
core.ignorecase=false), binary search ('cache_name_pos') may be better. The
difficulty is to decide when to choose one over the other :-)
> Maybe we could save it in a separate index extension, but we
> need to verify that the reader uses the same hash function as the
> writer.
>
>>> Similarly, the '--directory' option controls early returns from the
>>> directory scan (via read_directory_recursive's check_only argument), so you
>>> won't be able to get a full untracked files listing if the cache was
>>> recorded with '--directory'. Additionally, '--directory' aggregates the
>>> state at the topmost untracked directory, so that directory's cached state
>>> depends on all sub-directories as well...
>>
>> I missed this. We could ignore check_only if caching is enabled, but
>> that does not sound really good. Let me think about it more..
>
> We could save "check_only" to the cache as well. This way we don't
> have to disable the check_only trick completely.
>
> So we process a directory with check_only set, find one untracked
> entry and stop short. We store check_only value and the status ("found
> something") in addition to dir mtime. Next time we check the dir's
> mtime. If it matches and is called with check_only set, we know there
> is at least one untracked entry, that's enough to stop r_d_r and
> return early. If dir mtime does not match, or r_d_r is called without
> check_only, we ignore the cached data and fall back to opendir.
>
> Sounds good?
>
What about untracked files in sub-directories? E.g. you have untracked dirs a/b
with untracked file a/b/c, so normal 'git status' would list 'a/' as untracked.
Now, 'rm a/b/c' would update mtime of b, but not of a, so you'd still list 'a/'
as untracked. Same thing for 'echo "c" >a/b/.gitignore'.
Your solution could work if you additionally cache the directories that had to
be scanned to find that first untracked file (but you probably had that in mind
anyway).
If the cache is only used for certain dir_struct.flags combinations, you can
probably get around saving the check_only flag (which can only ever be true if
both DIR_SHOW_OTHER_DIRECTORIES and DIR_HIDE_EMPTY_DIRECTORIES are set (which
is the default for 'git status')).
--
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 2/2] git-svn: only look at the root path for svn:mergeinfo
Jakob Stoklund Olesen wrote: > Subversion can put mergeinfo on any sub-directory to track cherry-picks. > Since cherry-picks are not represented explicitly in git, git-svn should > just ignore it. Hi, was git-svn trying to track cherry-picks as merge before? This changes behavior a bit, so two independent users of git-svn may not have identical histories as a result, correct? Can you add a test to ensure this behavior is preserved? Thanks. Sorry, I've never looked at mergeinfo myself, mainly relying on Sam + tests for this. [1] - Historically, git-svn (using defaults) has always tried to preserve identical histories for independent users across different git-svn versions. However, mergeinfo may be enough of a corner-case where we can make an exception. -- 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 1/2] git-svn: only look at the new parts of svn:mergeinfo
Thanks! I still haven't gotten around to looking at svn:mergeinfo things, but this passes tests so I'm inclined to merge this unless somebody disagrees. -- 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 3/3] fetch.c: use a single ref transaction for all ref updates
Change store_updated_refs to use a single ref transaction for all refs that
are updated during the fetch. This makes the fetch more atomic when update
failures occur.
Since ref update failures will now no longer occur in the code path for
updating a single ref in s_update_ref, we no longer have as detailed error
message logging the exact reference and the ref log action as in the old code.
Instead since we fail the entire transaction we log a much more generic
message. But since we commit the transaction using MSG_ON_ERR we will log
an error containing the ref name if either locking of writing the ref would
so the regression in the log message is minor.
This will also change the order in which errors are checked for and logged
which may alter which error will be logged if there are multiple errors
occuring during a fetch.
For example, assuming we have a fetch for two refs that both would fail.
Where the first ref would fail with ENOTDIR due to a directory in the ref
path not existing, and the second ref in the fetch would fail due to
the check in update_logical_ref():
if (current_branch &&
!strcmp(ref->name, current_branch->name) &&
!(update_head_ok || is_bare_repository()) &&
!is_null_sha1(ref->old_sha1)) {
/*
* If this is the head, and it's not okay to update
* the head, and the old value of the head isn't empty...
*/
In the old code sicne we would update the refs one ref at a time we would
first fail the ENOTDIR and then fail the second update of HEAD as well.
But since the first ref failed with ENOTDIR we would eventually fail the whole
fetch with STORE_REF_ERROR_DF_CONFLICT
In the new code, since we defer committing the transaction until all refs
has been processed, we would now detect that the second ref was bad and
rollback the transaction before we would even try start writing the update to
disk and thus we would not return STORE_REF_ERROR_DF_CONFLICT for this case.
I think this new behaviour is more correct, since if there was a problem
we would not even try to commit the transaction but need to highlight this
change in how/what errors are reported.
This change in what error is returned only occurs if there are multiple
refs that fail to update and only some, but not all, of them fail due to
ENOTDIR.
Signed-off-by: Ronnie Sahlberg
---
builtin/fetch.c | 34 +++---
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 5c15584..97c6b9a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -45,6 +45,7 @@ static struct transport *gsecondary;
static const char *submodule_prefix = "";
static const char *recurse_submodules_default;
static int shown_url;
+struct ref_transaction *transaction;
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -373,24 +374,12 @@ static int s_update_ref(const char *action,
struct ref *ref,
int check_old)
{
- char msg[1024];
- char *rla = getenv("GIT_REFLOG_ACTION");
- struct ref_transaction *transaction;
-
if (dry_run)
return 0;
- if (!rla)
- rla = default_rla.buf;
- snprintf(msg, sizeof(msg), "%s: %s", rla, action);
- errno = 0;
- transaction = ref_transaction_begin();
- if (!transaction ||
- ref_transaction_update(transaction, ref->name, ref->new_sha1,
- ref->old_sha1, 0, check_old) ||
- ref_transaction_commit(transaction, msg, UPDATE_REFS_QUIET_ON_ERR))
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
+ if (ref_transaction_update(transaction, ref->name, ref->new_sha1,
+ ref->old_sha1, 0, check_old))
+ return STORE_REF_ERROR_OTHER;
return 0;
}
@@ -563,6 +552,13 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
goto abort;
}
+ errno = 0;
+ transaction = ref_transaction_begin();
+ if (!transaction) {
+ rc = error(_("cannot start ref transaction\n"));
+ goto abort;
+ }
+
/*
* We do a pass for each fetch_head_status type in their enum order, so
* merged entries are written before not-for-merge. That lets readers
@@ -674,6 +670,14 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
}
}
}
+ if (rc)
+ ref_transaction_rollback(transaction);
+ else {
+ if (ref_transaction_commit(transaction, "fetch_ref transaction",
+ UPDATE_REFS_MSG_ON_ERR))
+ rc |= errno == EN
[PATCH 2/3] fetch.c: change s_update_ref to use a ref transaction
Change s_update_ref to use a ref transaction for the ref update.
Signed-off-by: Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg
---
builtin/fetch.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a93c893..5c15584 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -375,7 +375,7 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv("GIT_REFLOG_ACTION");
- static struct ref_lock *lock;
+ struct ref_transaction *transaction;
if (dry_run)
return 0;
@@ -384,15 +384,14 @@ static int s_update_ref(const char *action,
snprintf(msg, sizeof(msg), "%s: %s", rla, action);
errno = 0;
- lock = lock_any_ref_for_update(ref->name,
- check_old ? ref->old_sha1 : NULL,
- 0, NULL);
- if (!lock)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- if (write_ref_sha1(lock, ref->new_sha1, msg) < 0)
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref->name, ref->new_sha1,
+ ref->old_sha1, 0, check_old) ||
+ ref_transaction_commit(transaction, msg, UPDATE_REFS_QUIET_ON_ERR))
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
+
return 0;
}
--
1.9.1.518.g16976cb.dirty
--
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 1/3] fetch.c: clear errno before calling functions that might set it
In s_update_ref there are two calls that when they fail we return an error based on the errno value. In particular we want to return a specific error if ENOTDIR happened. Both these functions do have failure modes where they may return an error without updating errno, in which case a previous and unrelated ENOTDIT may cause us to return the wrong error. Clear errno before calling any functions if we check errno afterwards. Also skip initializing a static variable to 0. Sstatics live in .bss and are all automatically initialized to 0. Signed-off-by: Ronnie Sahlberg --- builtin/fetch.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 55f457c..a93c893 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -44,7 +44,7 @@ static struct transport *gtransport; static struct transport *gsecondary; static const char *submodule_prefix = ""; static const char *recurse_submodules_default; -static int shown_url = 0; +static int shown_url; static int option_parse_recurse_submodules(const struct option *opt, const char *arg, int unset) @@ -382,6 +382,8 @@ static int s_update_ref(const char *action, if (!rla) rla = default_rla.buf; snprintf(msg, sizeof(msg), "%s: %s", rla, action); + + errno = 0; lock = lock_any_ref_for_update(ref->name, check_old ? ref->old_sha1 : NULL, 0, NULL); -- 1.9.1.518.g16976cb.dirty -- 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 0/3] Use ref transactions for fetch
This change is based on the previous ref transaction patches. This is sent as a separate patch series since it implements a lot more non-trivial changes to the behaviour than the previous patches and thus can use more detailed review. Update fetch.c to use ref transactions when performing updates. Use a single ref transaction for all updates and only commit the transaction if all other checks and oeprations have been successful. This makes the ref updates during a fetch (mostly) atomic. Ronnie Sahlberg (3): fetch.c: clear errno before calling functions that might set it fetch.c: change s_update_ref to use a ref transaction fetch.c: use a single ref transaction for all ref updates builtin/fetch.c | 39 ++- 1 file changed, 22 insertions(+), 17 deletions(-) -- 1.9.1.518.g16976cb.dirty -- 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: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1
On 2014-04-22 13:38, Junio C Hamano wrote:
> Michael Haggerty writes:
>
>> While we're at it, I think it would be prudent to ban '-' at the
>> beginning of reference name segments. For example, reference names like
>>
>> refs/heads/--cmd=/sbin/halt
>> refs/tags/--exec=forkbomb(){forkbomb|forkbomb&};forkbomb
>>
>> are currently both legal, but I think they shouldn't be.
>
> I think we forbid these at the Porcelain level ("git branch", "git
> checkout -b" and "git tag" should not let you create "-aBranch"),
> while leaving the plumbing lax to allow people experimenting with
> their repositories.
>
> It may be sensible to discuss and agree on what exactly should be
> forbidden (we saw "leading dash", "semicolon and dollar anywhere"
> so far in the discussion)
Also backquote anywhere.
> and plan for transition to forbid them
> everywhere in a next big version bump (it is too late for 2.0).
Would it be acceptable to have a config option to forbid these in a
non-major version bump? Does parsing config files add too much overhead
for this to be feasible?
If it's OK to have a config option, then here's one possible transition
path (probably flawed, but my intent is to bootstrap discussion):
1. Add an option to forbid dangerous characters. The option defaults
to disabled for compatibility. If the option is unset, print a
warning upon encountering a ref name that would be forbidden.
2. Later, flip the default to enabled.
3. Later, in the weeks/months leading up to the next major version
release, print the warning even if the config option is set to
disabled.
Thanks,
Richard
--
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 is missing from Git v2.0
Sebastian Schuberth writes: > On Mon, Apr 21, 2014 at 9:39 PM, Junio C Hamano wrote: > >>> If we don't standardize this now people will come up with their own >>> definitions [1] [2] (and many others if you just search GitHub) which >>> are again likely to differ (slightly), hindering interoperability. >> >> I am afraid that that ship has sailed long time ago, though. > > That's most likely true, but it does not get better by postponing this > even more. As I already said: I think it might be OK to implement them as the lowest priority fallback alias, so that '[alias] co = "user's definition"' anywhere in the various configuration locations will override it. I am a bit hesitant about adding start-up overhead, though. Also I am not sure if people can agree with (1) a broadly wide selection of aliases and (2) the actual definitions for them (I am OK with "co === checkout" myself, but I'd rather not to even think about my Git wasting cycles parsing extra configuration items to support "br === branch" at all, for example). I am not fundamentally opposed. I just do not think it would add much value to new people at this point, and it will actively hurt if we shoved barely cooked one in 2.0. A few thinking points that are necessary to be worked out, even before we start imagining a plan to introduce them, off the top of my head, are these: * Should we add many random aliases, or should we limit ourselves only to a narrow set that everybody can agree on? * What is the additional cognitive load do we feel comfortable burdening with the new users? * What is the exact mechanism to give these built-in aliases? Imagine that somebody says "[alias] ci = commit" and a handful of vocal people on this list agree that it is a good idea. Many random websites start mentioning "git ci -a" in their beginner examples. Users are led to think "ci" is a command just like "commit" and does the same thing. Some of them want to always commit everything before moving to their next task, and want to alias it further, e.g. "[alias] ci = ci -a"---which would not work. At that point, the users need to learn the distinction between native subcommands (e.g. "commit"), built-in fallback aliases (e.g. "ci") and aliases of their own in their ~/.gitconfig, and need to learn that only the first kind and not the second kind cannot be used to define their own alias (and the users need to learn "commit" at that time as well). That could be solved by making "ci" not a built-in fallback alias, but a new subcommand (then there is no "'foo' in 'git foo' could be a command or a built-in alias and they behave differently" issue), and additionally somehow allowing a native subcommand overriden by end-user alias, but I do not think anybody designed how such an override would work so far. So even if we agree that it would be a good idea to have some default fallback aliases, the set of such aliases we ship must be limited to a set that everybody can agree on, both in the sense that "adding alias XX is good" and also in the sense that "alias XX must be defined as YY". As you allueded to, the Git userbase is a lot larger than it used to be back in 2006, one alias, e.g. "[alias] br = branch", that is reported as either useless or needed to be further tweaked by a person on this list would mean that we would be either spending unnecessary start-up cycles (for "useless" case) or adding cognitive load of having to differente between "branch" and "br" (for "needs further tweak" case) for thousands of users who would be better off if we didn't have that specific alias. So while I understand the desire to have a bit more handholding and am not fundamentally opposed to the desire, I am not optimistic that an attempt to implement these "aliases" would result in a very useful addition to the system, even if done after careful thought. In any case, this definitely is not a 2.0 material. I agree that it would be good to start discussing it early (rather than later) if we ever want to do such a change. -- 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] tag: add -i and --introduced modifier for --contains
Jan Kara writes: > On Thu 17-04-14 10:04:52, Junio C Hamano wrote: >> So perhaps the rule should be updated to do something like: >> >> - find candidate tags that can be used to "describe --contains" >> the commit A, yielding v3.4, v3.5 (not shown), and v9.0; >> >> - among the candidate tags, cull the ones that contain another >> candidate tag, rejecting v3.5 (not shown) and v9.0; >> >> - among the surviving tags, pick the closest. > ... > Regarding the strategy what to select when there are several > remaining tags after first two steps I would prefer to output all such > tags. Yes, as I mentioned in another subthread ($gmane/246488), different projects want different tie-breaking rules at the third step, and your "show all to give more information to the user" could be another mode of operation. I offhand do not think the current name-rev machinery is set up to compute your variant easily, though. -- 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: gitignore vs. exclude vs assume-unchanged?
Andrew Ardill writes: > As a data point, I have seen people add ".gitignore" to their > .gitignore file, as they don't want to share the file. Interesting. It will break immediately when the project starts wanting to distribute its "canonical" ignore list, but until that time, it would "work" (for some definition of "working"). > It would be possible to check for this antipattern during normal use > and provide a hint to the user, but that is probably too heavy handed > and might annoy people with a legitimate use case. For that matter, if > the gitignore file is easier to use for the 'private ignore' use case > we have a bigger problem and shouldn't dictate to users what to use. Very true. > ... The introduction does specifically mention 'gitignore' > files, but that seems to be due to all the ignore files > ($HOME/.config/git/ignore, $GIT_DIR/info/exclude, .gitignore) being > classified as 'gitignore' files. Yes. Notice that that the blanket term used is not "dot-gitignore", but "gitignore". The difference may be too subtle, and your suggestion to introduce a new phrase "git ignore files" as the blanket term might be one way to make it less subtle. I would actually think "ignore files" (without Git, as all readers know we are talking about _our_ ignore mechanism in our documentation) may even be a better idea. > We could reference the multiple ignore locations earlier, for people > who don't read past the first paragraph of to documentation. > > "Git ignore files specify intentionally untracked files that Git > should ignore. A git ignore file can be specified for all local > repositories, a specific local repository, or shared with other users > of a repository. Files already tracked by Git are not affected; see > the NOTES below for details." Sounds good, with or without s/git ignore/ignore/. > Finally, it's a little confusing that one of the files is called 'exclude'. > > It would be great to rename it to 'ignore'; $GIT_DIR/info/exclude -> > $GIT_DIR/info/ignore. Is there any reason this shouldn't be done? If we were starting Git from scratch, we may have called it info/ignore, but we do not live in an ideal world, so we need to worry about people's existing repositories, scripts and templates. That does not mean we cannot transition over time, aiming to flip the default in a future big version bump (no, not in 2.0), along the lines of (note: I haven't thought this thru, and do not take this as an authoritative and correct plan): Step 1. - Check if info/ignore exists, and read it without reading or even checking the existence of info/exclude; - Check if info/exclude exists, and read it. Warn about future default change and tell the user to rename (or if we are absolutely sure that we are interactive, we can offer to do the rename for the user by prompting). Step 2. - Wait for several major releases, until major distros catch up with step 1. Step 3. - Drop the support for info/exclude altogether, without even warning about our stopping to read 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
Re: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1
Michael Haggerty writes:
> While we're at it, I think it would be prudent to ban '-' at the
> beginning of reference name segments. For example, reference names like
>
> refs/heads/--cmd=/sbin/halt
> refs/tags/--exec=forkbomb(){forkbomb|forkbomb&};forkbomb
>
> are currently both legal, but I think they shouldn't be.
I think we forbid these at the Porcelain level ("git branch", "git
checkout -b" and "git tag" should not let you create "-aBranch"),
while leaving the plumbing lax to allow people experimenting with
their repositories.
It may be sensible to discuss and agree on what exactly should be
forbidden (we saw "leading dash", "semicolon and dollar anywhere"
so far in the discussion) and plan for transition to forbid them
everywhere in a next big version bump (it is too late for 2.0).
--
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: [RTC/PATCH] Add 'update-branch' hook
Ilya Bobyr wrote: > On 4/22/2014 9:31 AM, Felipe Contreras wrote: > > Stephen Leake wrote: > >> Felipe Contreras writes: > >>> Yes, there a reason for the existance of those hooks. Now tell me why > >>> would > >>> anybody use post-update-branch instead of pre-update-branch? > >> > >> I have a branch which should always be recompiled on update; > >> post-update-branch would be a good place for that. > >> > > And why would pre-update-branch not serve that purpose? > > "pre-" hook could be used, but if the hooks is not supposed to prevent > the operation, it seems reasonable to put it in the "post-" hook should > one be available. If 'pre-update-branch' can be used, then you are pretty much agreeing to the fact that the 'post-udpate-branch' hook would be *useless*. Such a script would work both as 'pre-update-branch' and 'post-update-branch', therefore a single 'update-branch' would serve. So I ask again: Tell me why would anybody need 'post-update-branch' instead of 'pre-update-branch'? -- Felipe Contreras -- 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 2/2] mergetool: run prompt only if guessed tool
David Aguilar writes:
> [Cc:ing Charles in case he has an opinion, this behavior dates back to the
> original MT]
>
> On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:
>> It's annoying to see the prompt:
>>
>> Hit return to start merge resolution tool (foo):
>>
>> Every time the user does 'git mergetool' even if the user already
>> configured 'foo' as the wanted tool.
>>
>> Display this prompt only when the user hasn't explicitly configured a
>> tool.
>
> I agree this is probably helpful.
> Most users I've met end up configuring mergetool.prompt=false.
>
> Thanks
Do you have reaction to the other one "[PATCH 1/2] merge: enable
defaulttoupstream by default"?
As I do not think (note: I am not saying "I do not know" here) it is
sensible to do these "flip the default" in 2.0, I am hoping that we
can queue them (if the area maintainer, you, agree they are good
changes) to 'next' and cook them for the remainder of this cycle.
2.0 is a place where we execute the "flip the default" that we have
already disussed, designed and agreed since around 1.8.x timeframe,
and I would like to exclude any user visible change that just got
started being discussed to avoid unnecessary fallouts.
Thanks.
>
>> Signed-off-by: Felipe Contreras
>> ---
>> git-mergetool.sh | 14 +++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/git-mergetool.sh b/git-mergetool.sh
>> index 332528f..d08dc92 100755
>> --- a/git-mergetool.sh
>> +++ b/git-mergetool.sh
>> @@ -277,7 +277,7 @@ merge_file () {
>> echo "Normal merge conflict for '$MERGED':"
>> describe_file "$local_mode" "local" "$LOCAL"
>> describe_file "$remote_mode" "remote" "$REMOTE"
>> -if "$prompt" = true
>> +if test "$guessed_merge_tool" = true || test "$prompt" = true
>> then
>> printf "Hit return to start merge resolution tool (%s): "
>> "$merge_tool"
>> read ans || return 1
>> @@ -315,7 +315,8 @@ merge_file () {
>> return 0
>> }
>>
>> -prompt=$(git config --bool mergetool.prompt || echo true)
>> +prompt=$(git config --bool mergetool.prompt)
>> +guessed_merge_tool=false
>>
>> while test $# != 0
>> do
>> @@ -373,7 +374,14 @@ prompt_after_failed_merge () {
>>
>> if test -z "$merge_tool"
>> then
>> -merge_tool=$(get_merge_tool "$merge_tool") || exit
>> +# Check if a merge tool has been configured
>> +merge_tool=$(get_configured_merge_tool)
>> +# Try to guess an appropriate merge tool if no tool has been set.
>> +if test -z "$merge_tool"
>> +then
>> +merge_tool=$(guess_merge_tool) || exit
>> +guessed_merge_tool=true
>> +fi
>> fi
>> merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
>> merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries ||
>> echo false)"
>> --
>> 1.9.2+fc1.1.g5c924db
>>
>> --
>> 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
--
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] mergetools: add vimdiff3 mode
David Aguilar writes:
> On Sun, Apr 20, 2014 at 07:24:20PM -0500, Felipe Contreras wrote:
>> It's similar to the default, except that the other windows are hidden.
>> This ensures that removed/added colors are still visible on the main
>> merge window, but the other windows not visible.
>>
>> Specially useful with merge.conflictstyle=diff3.
>
> This is a nice addition, thanks.
>
> FWIW,
> Acked-by: David Aguilar
Thanks for an explicit Ack.
I personally think a new backend vimdiff3 can go in 2.0 (though we
are -rc0), as we can read from the patch below, it is very unlikely
to break anything else (I am not sure what these 'hid' are in the
implementation, but even if we had any breakage there, it would not
affect anybody other than vimdiff3 backend).
>>
>> Signed-off-by: Felipe Contreras
>> ---
>>
>> How a conflict looks:
>> http://felipec.org/vimdiff3-conflict.png
>>
>> How it looks resolved:
>> http://felipec.org/vimdiff3-resolved.png
>>
>> mergetools/gvimdiff3 | 1 +
>> mergetools/vimdiff | 14 --
>> mergetools/vimdiff3 | 1 +
>> 3 files changed, 14 insertions(+), 2 deletions(-)
>> create mode 100644 mergetools/gvimdiff3
>> create mode 100644 mergetools/vimdiff3
>>
>> diff --git a/mergetools/gvimdiff3 b/mergetools/gvimdiff3
>> new file mode 100644
>> index 000..04a5bb0
>> --- /dev/null
>> +++ b/mergetools/gvimdiff3
>> @@ -0,0 +1 @@
>> +. "$MERGE_TOOLS_DIR/vimdiff"
>> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
>> index 39d0327..1ddfbfc 100644
>> --- a/mergetools/vimdiff
>> +++ b/mergetools/vimdiff
>> @@ -20,16 +20,26 @@ merge_cmd () {
>> "$merge_tool_path" -f -d -c 'wincmd l' \
>> "$LOCAL" "$MERGED" "$REMOTE"
>> ;;
>> +gvimdiff3|vimdiff3)
>> +if $base_present
>> +then
>> +"$merge_tool_path" -f -d -c 'hid | hid | hid' \
>> +"$LOCAL" "$REMOTE" "$BASE" "$MERGED"
>> +else
>> +"$merge_tool_path" -f -d -c 'hid | hid' \
>> +"$LOCAL" "$REMOTE" "$MERGED"
>> +fi
>> +;;
>> esac
>> check_unchanged
>> }
>>
>> translate_merge_tool_path() {
>> case "$1" in
>> -gvimdiff|gvimdiff2)
>> +gvimdiff|gvimdiff2|gvimdiff3)
>> echo gvim
>> ;;
>> -vimdiff|vimdiff2)
>> +vimdiff|vimdiff2|vimdiff3)
>> echo vim
>> ;;
>> esac
>> diff --git a/mergetools/vimdiff3 b/mergetools/vimdiff3
>> new file mode 100644
>> index 000..04a5bb0
>> --- /dev/null
>> +++ b/mergetools/vimdiff3
>> @@ -0,0 +1 @@
>> +. "$MERGE_TOOLS_DIR/vimdiff"
>> --
>> 1.9.2+fc1.1.g5c924db
--
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 2/2] mergetool: run prompt only if guessed tool
David Aguilar writes:
> [Cc:ing Charles in case he has an opinion, this behavior dates back to the
> original MT]
>
> On Sun, Apr 20, 2014 at 07:17:34PM -0500, Felipe Contreras wrote:
>> It's annoying to see the prompt:
>>
>> Hit return to start merge resolution tool (foo):
>>
>> Every time the user does 'git mergetool' even if the user already
>> configured 'foo' as the wanted tool.
>>
>> Display this prompt only when the user hasn't explicitly configured a
>> tool.
>
> I agree this is probably helpful.
> Most users I've met end up configuring mergetool.prompt=false.
>
> Thanks
OK, is it an Ack?
Thanks for CC'ing Charles, by the way. I think his point about
mentioning the change of default somewhere in the documentation
has some merits, and it can be done in a follow-up patch on top.
>> Signed-off-by: Felipe Contreras
>> ---
>> git-mergetool.sh | 14 +++---
>> 1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/git-mergetool.sh b/git-mergetool.sh
>> index 332528f..d08dc92 100755
>> --- a/git-mergetool.sh
>> +++ b/git-mergetool.sh
>> @@ -277,7 +277,7 @@ merge_file () {
>> echo "Normal merge conflict for '$MERGED':"
>> describe_file "$local_mode" "local" "$LOCAL"
>> describe_file "$remote_mode" "remote" "$REMOTE"
>> -if "$prompt" = true
>> +if test "$guessed_merge_tool" = true || test "$prompt" = true
>> then
>> printf "Hit return to start merge resolution tool (%s): "
>> "$merge_tool"
>> read ans || return 1
>> @@ -315,7 +315,8 @@ merge_file () {
>> return 0
>> }
>>
>> -prompt=$(git config --bool mergetool.prompt || echo true)
>> +prompt=$(git config --bool mergetool.prompt)
>> +guessed_merge_tool=false
>>
>> while test $# != 0
>> do
>> @@ -373,7 +374,14 @@ prompt_after_failed_merge () {
>>
>> if test -z "$merge_tool"
>> then
>> -merge_tool=$(get_merge_tool "$merge_tool") || exit
>> +# Check if a merge tool has been configured
>> +merge_tool=$(get_configured_merge_tool)
>> +# Try to guess an appropriate merge tool if no tool has been set.
>> +if test -z "$merge_tool"
>> +then
>> +merge_tool=$(guess_merge_tool) || exit
>> +guessed_merge_tool=true
>> +fi
>> fi
>> merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)"
>> merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries ||
>> echo false)"
>> --
>> 1.9.2+fc1.1.g5c924db
>>
>> --
>> 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
--
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: [RTC/PATCH] Add 'update-branch' hook
On 4/22/2014 9:31 AM, Felipe Contreras wrote: > Stephen Leake wrote: >> Felipe Contreras writes: >> >>> Ilya Bobyr wrote: On 4/21/2014 2:17 PM, Felipe Contreras wrote: > Ilya Bobyr wrote: > >> Also, most have names that start with either "pre-" or "post-". >> It seems reasonable for both "pre-update-branch" and >> "post-update-branch" to exist. > I don't see what would be the point in that. Do you see the point in the other hooks doing that? >>> Yes, there a reason for the existance of those hooks. Now tell me why would >>> anybody use post-update-branch instead of pre-update-branch? >> I have a branch which should always be recompiled on update; >> post-update-branch would be a good place for that. > And why would pre-update-branch not serve that purpose? "pre-" hook could be used, but if the hooks is not supposed to prevent the operation, it seems reasonable to put it in the "post-" hook should one be available. For example, for clone and branch that would mean that that the branch sections are already created in .git/config, but for "pre-" hooks, should be find the right spot, configuration could probably be absent just yet. I do not think that someone is objecting adding just the "pre-" hook first. But it seems unlikely that one can envision all the possible use cases to say that "post-" hook would never be useful. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] send-email: recognize absolute path on Windows
Erik Faye-Lund writes:
>>> Shouldn't the latter also be anchored at the beginning of the string
>>> with a leading "^"?
>>>
+}
+
+require File::Spec::Functions;
+return File::Spec::Functions::file_name_is_absolute($path);
>>>
>>> We already "use File::Spec qw(something else)" at the beginning, no?
>>> Why not throw file_name_is_absolute into that qw() instead?
>>
>> Ahh, OK, if you did so, you won't have any place to hook the "only
>> on msys do this" trick into.
>>
>> It somehow feels somewhat confusing that we define a sub with the
>> same name as the system one, while not overriding it entirely but
>> delegate back to the system one. I am debating myself if it is more
>> obvious if it is done this way:
>>
>> use File::Spec::Functions qw(file_name_is_absolute);
>> if ($^O eq 'msys') {
>> sub file_name_is_absolute {
>> return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
>> }
>> }
>>
>
> In this case, we end up requiring that module even when we end up
> using it, no?
Also somebody earlier mentioned that we would be redefining, which
has a different kind of ugliness, so I'd agree with the code structure
of what you sent out (which has been queued on 'pu').
My earlier question "don't we want to make sure 'C:' is at the
betginning of the string?" still stands, though. I do not think I
futzed with your regexp in the version I queued on 'pu'.
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: Fwd: git p4: feature request - branch check filtering
Dan Porter writes: > I should have updated on this earlier, but I wished to refine my work > on this feature before submitting. With 2.0 looming I'll submit > what's there so far. I am not Pete, but... The pre-release time is to find and fix regressions that may have been introduced since the last release while we tried to add new features and fixes to 2.0 until now. "With 2.0 looming" is not a good reason to send out a new work. In fact, if 2.0 is "looming", it is already too late for the upcoming release. Of course "With 2.0 looming" does not mean that you must not work on things that are regression fixes---it is your own time and effort, and it will be great if that can help later releases. Thanks for contributing anyway ;-) -- 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: [RTC/PATCH] Add 'update-branch' hook
Stephen Leake wrote: > Felipe Contreras writes: > > > Ilya Bobyr wrote: > >> On 4/21/2014 2:17 PM, Felipe Contreras wrote: > >> > Ilya Bobyr wrote: > >> > > >> >> Also, most have names that start with either "pre-" or "post-". > >> >> It seems reasonable for both "pre-update-branch" and > >> >> "post-update-branch" to exist. > >> > I don't see what would be the point in that. > >> > >> Do you see the point in the other hooks doing that? > > > > Yes, there a reason for the existance of those hooks. Now tell me why would > > anybody use post-update-branch instead of pre-update-branch? > > I have a branch which should always be recompiled on update; > post-update-branch would be a good place for that. And why would pre-update-branch not serve that purpose? -- Felipe Contreras -- 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: [RTC/PATCH] Add 'update-branch' hook
Ilya Bobyr wrote: > On 4/21/2014 1:49 PM, Felipe Contreras wrote: > > Ilya Bobyr wrote: > >> On 4/20/2014 7:23 PM, Felipe Contreras wrote: > >>> This hook is invoked whenever a branch is updated, either when a branch > >>> is created or updated with 'git branch', or when it's rebased with 'git > >>> rebase'. It receives two parameters; the name of the branch, and the > >>> SHA-1 of the latest commit, additionally, if there was a base commit the > >>> branch was rebased onto, a third parameter contains it. > >> And the old branch SHA could be found from in the reflog, correct? > > Actually the old branch SHA-1 is actually the current one, since the branch > > hasn't been updated at that point. Personally I don't see much value in > > adding > > something the script can easily find out. > > If the hook is about a branch update, I would expect it to provide both > old and new points for the branch, along with the name. Again, I don't see the the point of passing something that is easy to find out: `git rev-parse $branch` gives you that information. > The fact that for rebases it also provides new base SHA is very > convenient. As it is an optional argument it may make further extension > of the interface a bit awkward. > So, is seems reasonable to provide both from the very beginning. So basically `git branch` would send the same SHA-1 twice. -- Felipe Contreras -- 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: [RTC/PATCH] Add 'update-branch' hook
Ilya Bobyr wrote:
> On 4/20/2014 7:23 PM, Felipe Contreras wrote:
> > [...]
> >
> > diff --git a/branch.c b/branch.c
> > index 660097b..c2058d1 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -4,6 +4,7 @@
> > #include "refs.h"
> > #include "remote.h"
> > #include "commit.h"
> > +#include "run-command.h"
> >
> > struct tracking {
> > struct refspec spec;
> > @@ -304,6 +305,11 @@ void create_branch(const char *head,
> > if (real_ref && track)
> > setup_tracking(ref.buf + 11, real_ref, track, quiet);
> >
> > + if (run_hook_le(NULL, "update-branch", ref.buf + 11, sha1_to_hex(sha1),
> > NULL)) {
> > + unlock_ref(lock);
>
> lock is NULL if dont_change_ref is true. unlock_ref() would crash in
> that case.
> You may want to add a test for that.
That should be easy to fix.
> > + die("hook 'update-branch' returned error");
> > + }
> > +
> > if (!dont_change_ref)
> > if (write_ref_sha1(lock, sha1, msg) < 0)
> > die_errno(_("Failed to write ref"));
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 9b3c04d..6ec96e5 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -581,9 +581,10 @@ static void update_remote_refs(const struct ref *refs,
> > }
> > }
> >
> > -static void update_head(const struct ref *our, const struct ref *remote,
> > +static int update_head(const struct ref *our, const struct ref *remote,
> > const char *msg)
> > {
> > + int err = 0;
> > if (our && starts_with(our->name, "refs/heads/")) {
> > /* Local default branch link */
> > create_symref("HEAD", our->name, NULL);
> > @@ -591,6 +592,7 @@ static void update_head(const struct ref *our, const
> > struct ref *remote,
> > const char *head = skip_prefix(our->name,
> > "refs/heads/");
> > update_ref(msg, "HEAD", our->old_sha1, NULL, 0,
> > DIE_ON_ERR);
> > install_branch_config(0, head, option_origin,
> > our->name);
> > + err = run_hook_le(NULL, "update-branch", head,
> > sha1_to_hex(our->old_sha1), NULL);
>
> This is happening after the branch is updated and a config section for
> it is created.
I see that now, however, I cannot find where in builtin/clone.c is the branch
ref actually updated.
Worst, I don't see how I could possibly configure a hook to be triggered when
cloning, so I cannot test.
> > }
> > } else if (our) {
> > struct commit *c = lookup_commit_reference(our->old_sha1);
> > @@ -606,6 +608,7 @@ static void update_head(const struct ref *our, const
> > struct ref *remote,
> > update_ref(msg, "HEAD", remote->old_sha1,
> >NULL, REF_NODEREF, DIE_ON_ERR);
> > }
> > + return err;
> > }
> >
> > static int checkout(void)
> > @@ -987,7 +990,7 @@ int cmd_clone(int argc, const char **argv, const char
> > *prefix)
> > update_remote_refs(refs, mapped_refs, remote_head_points_at,
> >branch_top.buf, reflog_msg.buf, transport,
> > !is_local);
> >
> > - update_head(our_head_points_at, remote_head, reflog_msg.buf);
> > + err = update_head(our_head_points_at, remote_head, reflog_msg.buf);
> >
> > transport_unlock_pack(transport);
> > transport_disconnect(transport);
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 1c41cbd..084dc36 100644
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -631,7 +631,11 @@ do_next () {
> > git update-ref -m "$message" $head_name $newhead $orig_head &&
> > git symbolic-ref \
> > -m "$GIT_REFLOG_ACTION: returning to $head_name" \
> > - HEAD $head_name
> > + HEAD $head_name &&
> > + if test -x "$GIT_DIR"/hooks/update-branch; then
> > + "$GIT_DIR"/hooks/update-branch $branch_name \
> > + $newhead $onto
> > + fi
>
> It looks like this is also after the branch was already updated.
This and the one below should be easy to fix.
--
Felipe Contreras
--
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: [NOT_A_PATCH] A naive attempt to cross-build Linux->mingw64 Git
Marat Radchenko wrote: > On Mon, Apr 21, 2014 at 07:06:24PM -0500, Felipe Contreras wrote: > > I managed to fix all the errors, some apply to newer mingw, regardless of > > 32 or > > 64, others are specific to 64-bit. It's all hacky and I haven't checked if > > it > > runs, but at least it compiles (mostly). > > Do you plan to evolve it into something mergeable? I might, but that would requiere a lot of effort to investigate the changes in mingw, and I'm not sure if there's enough interest on this. After fixing all the issues so that we can cross-compile, I would like to see a real effort to move away from shell and perl scripts, so that we not only could run the important commands, but properly test Git without bash, otherwise I feel Windows will always be a second class citizen. > P.S. besides CC/LD, I also had to define AR and fix `windres` executable name > in `config.mak.uname`. That's why we should have a CROSS_COMPILE variable which is standard in other projects. This is what the Linux kernel has: AS= $(CROSS_COMPILE)as LD= $(CROSS_COMPILE)ld CC= $(CROSS_COMPILE)gcc AR= $(CROSS_COMPILE)ar NM= $(CROSS_COMPILE)nm STRIP = $(CROSS_COMPILE)strip OBJCOPY = $(CROSS_COMPILE)objcopy OBJDUMP = $(CROSS_COMPILE)objdump I had patches for this, but I gave them up =/ -- Felipe Contreras -- 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: Project idea: github-like diff view
Duy Nguyen wrote: > On Sun, Apr 20, 2014 at 9:46 PM, Jeff King wrote: > > On Sun, Apr 20, 2014 at 04:58:28PM +0700, Duy Nguyen wrote: > > > >> - --color-words within unified diff format, using background color to > >> show what part of the line has changed. This is only enabled for > >> 1-line changes. > > > > See contrib/diff-highlight. > > Thanks. I'd rather have it built in core git still. I'll try to see if > I can rewrite it in C. Else, any objection to promote it to a core > helper and setup pager automatically? We can have a config key to turn > it off, but if git diff is colored, then it could be on by default. Having so many tools that should be rewritten to C, I don't see why anybody should spent time rewriting scripts that are not part of the core and for the most part do their job already. -- Felipe Contreras -- 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: [msysGit] Re: Re: [ANNOUNCE] WinGit - native x86/x64 Git for Windows
Hi Felipe, On Tue, 22 Apr 2014, Felipe Contreras wrote: > Johannes Schindelin wrote: > > On Mon, 21 Apr 2014, Felipe Contreras wrote: > > > Johannes Schindelin wrote: > > > > Now, clearly you have all the motivation that is needed to get 64-bit > > > > builds of Git for Windows going, and all the motivation required to make > > > > sure that the MSVC support of the msysGit project works. > > > > > > s/msysGit/Git/ > > > > No. I meant the msysGit project; the project that maintains the current > > development environment for Git for Windows. Please do not try to > > reinterpret what I am saying. > > I don't care what you are saying, That is quite obvious and did not need clarifying. Nevertheless, by stating that "substitute" command, you corrected me. Except that you did not, I intended to say something different, and your correction was quite misplaced. Ciao, Johannes -- 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: [msysGit] Re: Re: [ANNOUNCE] WinGit - native x86/x64 Git for Windows
Johannes Schindelin wrote: > On Mon, 21 Apr 2014, Felipe Contreras wrote: > > Johannes Schindelin wrote: > > > Now, clearly you have all the motivation that is needed to get 64-bit > > > builds of Git for Windows going, and all the motivation required to make > > > sure that the MSVC support of the msysGit project works. > > > > s/msysGit/Git/ > > No. I meant the msysGit project; the project that maintains the current > development environment for Git for Windows. Please do not try to > reinterpret what I am saying. I don't care what you are saying, what I'm saying is that he has the motivation to do it for the Git project. Why on Earth would he do it for the msysGit project, when he can do it for the Git project (which would then percolate to the msysGit project as well)? > > Personally I don't see why ideally I shouldn't be able to build upstream > > Git for Windows with mingw without leaving my Linux system. > > Maybe because you could not even test it properly, let alone run the test > suite? Ideally you could. > And maybe because according to the famous "scratch your own itch" > credo, it is actually the duty of Windows users -- i.e. users who do not > even have your Linux system -- to fix the bugs that would never be > encountered anywhere but Windows? That is your conclusion. If somebody reasonable sends reasonable patches that make WinGit work in a way that doesn't impact negatively non-Windows users, I don't see why they wouldn't be picked. -- Felipe Contreras -- 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: Project idea: github-like diff view
On Tue, Apr 22, 2014 at 04:59:17PM +0700, Duy Nguyen wrote: > On Sun, Apr 20, 2014 at 9:46 PM, Jeff King wrote: > > On Sun, Apr 20, 2014 at 04:58:28PM +0700, Duy Nguyen wrote: > > > >> - --color-words within unified diff format, using background color to > >> show what part of the line has changed. This is only enabled for > >> 1-line changes. > > > > See contrib/diff-highlight. > > Thanks. I'd rather have it built in core git still. I'll try to see if > I can rewrite it in C. Else, any objection to promote it to a core > helper and setup pager automatically? We can have a config key to turn > it off, but if git diff is colored, then it could be on by default. If you are going to write it as part of git, it would be interesting to try using a real word-diff to find the inter-line changes, instead of the "front and back match" heuristic that the script uses. I know there are some cases that would look better, like: -foo(buf, len); +foo(obj->buf, obj->len); but I suspect some cases would also look worse. It would be interesting to experiment with, though. -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: [RTC/PATCH] Add 'update-branch' hook
Felipe Contreras writes: > Ilya Bobyr wrote: >> On 4/21/2014 2:17 PM, Felipe Contreras wrote: >> > Ilya Bobyr wrote: >> > >> >> Also, most have names that start with either "pre-" or "post-". >> >> It seems reasonable for both "pre-update-branch" and >> >> "post-update-branch" to exist. >> > I don't see what would be the point in that. >> >> Do you see the point in the other hooks doing that? > > Yes, there a reason for the existance of those hooks. Now tell me why would > anybody use post-update-branch instead of pre-update-branch? I have a branch which should always be recompiled on update; post-update-branch would be a good place for that. -- -- Stephe -- 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 v4 2/2] blame: dynamic blame_date_width for different locales
When show date in relative date format for git-blame, the max display
width of datetime is set as the length of the string "Thu Oct 19
16:00:04 2006 -0700" (30 characters long). But actually the max width
for C locale is only 22 (the length of string "x years, xx months ago").
And for other locale, it maybe smaller. E.g. For Chinese locale, only
needs a half (16-character width).
Set blame_date_width as the display width of _("4 years, 11 months
ago"), so that translators can make the choice.
Helped-by: Junio C Hamano
Signed-off-by: Jiang Xin
---
builtin/blame.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 35e95db..128fc64 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2338,7 +2338,14 @@ parse_done:
blame_date_width = sizeof("2006-10-19");
break;
case DATE_RELATIVE:
- /* "normal" is used as the fallback for "relative" */
+ /* TRANSLATORS: This string is used to tell us the maximum
+ display width for a relative timestamp in "git blame"
+ output. For C locale, "4 years, 11 months ago", which
+ takes 22 places, is the longest among various forms of
+ relative timestamps, but your language may need more or
+ fewer display columns. */
+ blame_date_width = utf8_strwidth(_("4 years, 11 months ago")) +
1; /* add the null */
+ break;
case DATE_LOCAL:
case DATE_NORMAL:
blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
--
1.9.2.476.gff10cf3.dirty
--
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 v4 1/2] bugfix: fix broken time_buf paddings for git-blame
Command `git blame --date relative` aligns the date field with a
fixed-width (defined by blame_date_width), and if time_str is shorter
than that, it adds spaces for padding. But there are two bugs in the
following codes:
time_len = strlen(time_str);
...
memset(time_buf + time_len, ' ', blame_date_width - time_len);
1. The type of blame_date_width is size_t, which is unsigned. If
time_len is greater than blame_date_width, the result of
"blame_date_width - time_len" will never be a negative number, but a
really big positive number, and will cause memory overwrite.
This bug can be triggered if either l10n message for function
show_date_relative() in date.c is longer than 30 characters, then
`git blame --date relative` may exit abnormally.
2. When show blame information with relative time, the UTF-8 characters
in time_str will break the alignment of columns after the date field.
This is because the time_buf padding with spaces should have a
constant display width, not a fixed strlen size. So we should call
utf8_strwidth() instead of strlen() for width calibration.
Helped-by: Nguyễn Thái Ngọc Duy
Helped-by: Eric Sunshine
Signed-off-by: Jiang Xin
---
builtin/blame.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 88cb799..35e95db 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1556,22 +1556,29 @@ static void assign_blame(struct scoreboard *sb, int opt)
static const char *format_time(unsigned long time, const char *tz_str,
int show_raw_time)
{
- static char time_buf[128];
+ static struct strbuf time_buf = STRBUF_INIT;
+ strbuf_reset(&time_buf);
if (show_raw_time) {
- snprintf(time_buf, sizeof(time_buf), "%lu %s", time, tz_str);
+ strbuf_addf(&time_buf, "%lu %s", time, tz_str);
}
else {
const char *time_str;
- int time_len;
+ size_t time_width;
int tz;
tz = atoi(tz_str);
time_str = show_date(time, tz, blame_date_mode);
- time_len = strlen(time_str);
- memcpy(time_buf, time_str, time_len);
- memset(time_buf + time_len, ' ', blame_date_width - time_len);
+ strbuf_addstr(&time_buf, time_str);
+ /*
+* Add space paddings to time_buf to display a fixed width
+* string, and use time_width for display width calibration.
+*/
+ for (time_width = utf8_strwidth(time_str);
+time_width < blame_date_width;
+time_width++)
+ strbuf_addch(&time_buf, ' ');
}
- return time_buf;
+ return time_buf.buf;
}
#define OUTPUT_ANNOTATE_COMPAT 001
--
1.9.2.476.gff10cf3.dirty
--
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 v4 0/2] peroper align of datetime filed of git-blame
Changes since V3: * rollback patch 2/2 to v2, but with a nicer comment for translators. Jiang Xin (2): bugfix: fix broken time_buf paddings for git-blame blame: dynamic blame_date_width for different locales builtin/blame.c | 30 ++ 1 file changed, 22 insertions(+), 8 deletions(-) -- 1.9.2.476.gff10cf3.dirty -- 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: [ANNOUNCE] WinGit - native x86/x64 Git for Windows
Johannes Schindelin writes:
> On Mon, 21 Apr 2014, Felipe Contreras wrote:
>
>> Johannes Schindelin wrote:
>> > Now, clearly you have all the motivation that is needed to get 64-bit
>> > builds of Git for Windows going, and all the motivation required to make
>> > sure that the MSVC support of the msysGit project works.
>>
>> s/msysGit/Git/
>
> No. I meant the msysGit project; the project that maintains the current
> development environment for Git for Windows. Please do not try to
> reinterpret what I am saying.
>
>> Personally I don't see why ideally I shouldn't be able to build upstream
>> Git for Windows with mingw without leaving my Linux system.
>
> Maybe because you could not even test it properly, let alone run the test
> suite? And maybe because according to the famous "scratch your own itch"
> credo, it is actually the duty of Windows users -- i.e. users who do not
> even have your Linux system -- to fix the bugs that would never be
> encountered anywhere but Windows?
http://www.lilypond.org/gub>
The LilyPond project uses this to do automated builds for Windows,
MacOSX, FreeBSD, GNU/Linux on several CPUs. The installation includes a
Python interpreter, GUILE, bash, and some other run-time necessary stuff
for executing scripts of various kinds.
LilyPond contains quite a few dependencies: efforts to do this natively
under the "everything that should be necessary is available somewhere"
assumptions led to bugs and time lags not dissimilar to what plagues
msysGit.
"duty of Windows users" sounds like a theory expounded by non-Windows
users. Maintaining ports requires highly skilled programmers, and
highly skilled programmers tend to scratch a _lot_ of itches by not
using Windows in the first place.
It's been a long time since I had a grasp of the Windows/Git situation,
but my impression was that much of the msysGit stuff was done by you
yourself to scratch your personal itch of stopping people to say "Git is
not useful for large projects since it does not run under Windows" while
not actually being a Windows user yourself.
So if my memory does not do me a disfavor, you have kicked the "duty of
Windows users" theory in the curb yourself.
The developer demographic of LilyPond is similar: we actually have a
predominance of Windows users on the user mailing list. But power users
and compile farm providers (all the cross-compiling is taking a serious
toll, even though most is in compiling the embedded example images in
the various manuals and their translations) use GNU/Linux, and where
their native system is Windows, in the form of a Ubuntu VM ("LilyDev")
put together for that purpose.
As a consequence, the bug tracker contains comparatively few and often
minor operating system specific bug reports (cf
http://code.google.com/p/lilypond/issues/list?can=1&q=OpSys%3DWindows>).
Many of them are catered for by programmers not even having a system
available for testing. Stuff that is really only reproducible on
Windows tends to take longer to fix. That involves things like
Font handling, PDF handling, filename issues, memory allocation handling
and others, often in the form of performance regressions that also
happen on GNU/Linux but are much less noticeable because the respective
facilities are much more efficient and thus mask unnecessarily repeated
operations much better.
While the user demographic of Git is likely leaning less towards Windows
than that of LilyPond, I expect some similar tendencies. As a result of
the GUB crosscompiling environment, LilyPond offers a high quality
up-to-date Windows distribution with a somewhat typical installer
(though with acceptability problems that would not be dissimilar for
Git, cf
http://download.cnet.com/LilyPond/9241-2141_4-10995890.html?messageID=10589553&tag=uo;uo>).
In a way, using such a cross-building environment is a copout regarding
the defensible "duty of end users" line of thought. But it's not like
the msysGit history supports that theory all that convincingly anyway.
--
David Kastrup
--
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: [NOT_A_PATCH] A naive attempt to cross-build Linux->mingw64 Git
Hi Marat, On Tue, 22 Apr 2014, Marat Radchenko wrote: > Johannes says building mingw64 Git is dirt-easy. Marat, please let's stop misquoting me, okay? What I said was more along the lines that there had been some start of a work on getting things to compile for 64-bit Windows, but that the test suite did not pass. Even cutting out the part about the test suite from quoting me leaves out the main point of what I said. And for the record: I just had a look; the beginnings of W64 support are in https://github.com/msysgit/git/compare/7f37564...work/w64. And again for the record: at least from my side, there is more than just willingness to cooperate. We'd just need to start a conversation in the second person (as opposed to the third person). Ciao, Johannes -- 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: [msysGit] Re: Re: [ANNOUNCE] WinGit - native x86/x64 Git for Windows
Hi, On Mon, 21 Apr 2014, Felipe Contreras wrote: > Johannes Schindelin wrote: > > Now, clearly you have all the motivation that is needed to get 64-bit > > builds of Git for Windows going, and all the motivation required to make > > sure that the MSVC support of the msysGit project works. > > s/msysGit/Git/ No. I meant the msysGit project; the project that maintains the current development environment for Git for Windows. Please do not try to reinterpret what I am saying. > Personally I don't see why ideally I shouldn't be able to build upstream > Git for Windows with mingw without leaving my Linux system. Maybe because you could not even test it properly, let alone run the test suite? And maybe because according to the famous "scratch your own itch" credo, it is actually the duty of Windows users -- i.e. users who do not even have your Linux system -- to fix the bugs that would never be encountered anywhere but Windows? Hth, Johannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] blame: use a helper to get suitable blame_date_width
2014-04-22 3:19 GMT+08:00 Junio C Hamano :
> Junio C Hamano writes:
>
>> What I am wondering is if we can do something
>> like this:
>> ...
>
> Nah, that is a lot more stupid than just doing
>
>> In code:
>>
>> blame_date_width = strtoul(_("4 years, 11 months ago"), NULL, 10) +
>> 1;
>>
>> In git.pot:
>>
>> #. This string is used to tell us the maximum display width for a
>> #. relative timestamp in "git blame" output. For C locale, "4 years,
>> #. 11 months ago", which takes 22 places, is the longest among
>> various
>> #. forms of relative timestamps, but your language may need more or
>> #. fewer display columns.
>> msgid "4 years, 11 months ago"
>> msgstr ""
>>
>> In de.po:
>> #. This string is used to tell us the maximum display width for a
>> #. relative timestamp in "git blame" output. For C locale, "4 years,
>> #. 11 months ago", which takes 22 places, is the longest among
>> various
>> #. forms of relative timestamps, but your language may need more or
>> #. fewer display columns.
>> msgid "4 years, 11 months ago"
>> msgstr ""vor 4 Jahren, und 11 Monaten"
>
> which is essentially how your very original looked like (modulo the
> comments). So let's not try to be clever or cute, and just have a
> good instruction in the TRANSLATORS comments.
>
> Sorry for flipping and flopping on this one.
I will send patch v4 tonight, and I think all l10n team leaders should
pay attention
to this thread:
* http://thread.gmane.org/gmane.comp.version-control.git/246464
--
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] send-email: recognize absolute path on Windows
On Wed, Apr 16, 2014 at 7:19 PM, Junio C Hamano wrote:
> Junio C Hamano writes:
>
>> Erik Faye-Lund writes:
>>
>>> So let's manually check for these in that case, and fall back to
>>> the File::Spec-helper on other platforms (e.g Win32 with native
>>> Perl)
>>> ...
>>> +sub file_name_is_absolute {
>>> +my ($path) = @_;
>>> +
>>> +# msys does not grok DOS drive-prefixes
>>> +if ($^O eq 'msys') {
>>> +return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
>>
>> Shouldn't the latter also be anchored at the beginning of the string
>> with a leading "^"?
>>
>>> +}
>>> +
>>> +require File::Spec::Functions;
>>> +return File::Spec::Functions::file_name_is_absolute($path);
>>
>> We already "use File::Spec qw(something else)" at the beginning, no?
>> Why not throw file_name_is_absolute into that qw() instead?
>
> Ahh, OK, if you did so, you won't have any place to hook the "only
> on msys do this" trick into.
>
> It somehow feels somewhat confusing that we define a sub with the
> same name as the system one, while not overriding it entirely but
> delegate back to the system one. I am debating myself if it is more
> obvious if it is done this way:
>
> use File::Spec::Functions qw(file_name_is_absolute);
> if ($^O eq 'msys') {
> sub file_name_is_absolute {
> return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
> }
> }
>
In this case, we end up requiring that module even when we end up
using it, no? Not that I have very strong objections for doing just
that, after all, it appears to be built-in. (As you might understand
from this message, my perl-fu is really lacking :-P)
--
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 2/2] blame: use different blame_date_width for different locale
2014-04-22 18:01 GMT+08:00 David Kastrup :
> Junio C Hamano writes:
>
>> This is not wrong per-se, but I am not sure if translators would
>> understand that "years and months ago" may not be the longuest
>> variant for their language and they are asked to use "89 seconds
>> ago" if the translation of that is longer than the translation for
>> "4 years, 11 months ago" in their language, with the given
>> explanation.
>
> What's with the 89? And the other semi-magic numbers?
Not something magic, just what show_date_relative() in date.c is implemented:
98 diff = now->tv_sec - time;
99 if (diff < 90) {
100 strbuf_addf(timebuf,
101 Q_("%lu second ago", "%lu seconds
ago", diff), diff);
102 return;
103 }
104 /* Turn it into minutes */
105 diff = (diff + 30) / 60;
106 if (diff < 90) {
107 strbuf_addf(timebuf,
108 Q_("%lu minute ago", "%lu minutes
ago", diff), diff);
109 return;
110 }
--
Jiang Xin
--
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: Open Source Organizational Culture
On 4/22/2014 12:26 AM, Felipe Contreras wrote: > Marius Storm-Olsen wrote: >> Turns out the osocas.2014.sgizmo.com survey subdomain gives an SSL >> warning for the *.sgizmo.com certificate. *face palm* >> >> Feel free to use >> http://bit.ly/OSOCAS2014 >> instead, which will redirect to the non-subdomain version >> https://www.surveygizmo.com/s3/1587798/osocas-2014 >> >> Sorry for the inconvenience(s)! > > I already filled the survey in the first URL, will that work? No problem, all the links work, just that the two in the original email are "noisy". Thank you for your participation! -- .marius -- 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: [ANNOUNCE] Git v2.0.0-rc0
On Apr 18, 2014, at 12:37, Junio C Hamano wrote:
> An early preview release Git v2.0.0-rc0 is now available for
> testing at the usual places.
I have run into the following test failures with v2.0.0-rc0:
Test Summary Report
---
t9117-git-svn-init-clone.sh (Wstat: 256 Tests: 11 Failed: 2)
Failed tests: 10-11
Non-zero exit status: 1
Files=658, Tests=11878, 3684 wallclock secs
Result: FAIL
The cause of the failure in both tests is this:
$ git svn init -s "$svnrepo"/project project --prefix=""
Option prefix requires an argument
The problem with --prefix="" is this (from the Getopt::Long CHANGES file):
Changes in version 2.37
---
* Bugfix: With gnu_compat, --foo= will no longer trigger "Option
requires an argument" but return the empty string.
The system I ran the tests on happens to have Getopt::Long version 2.35.
The --prefix="" option can be rewritten --prefix "" in both tests and then
they pass.
Alternatively this change can be made in git-svn.perl:
|diff --git a/git-svn.perl b/git-svn.perl
|index 7349ffea..284f458a 100755
|--- a/git-svn.perl
|+++ b/git-svn.perl
|@@ -149,7 +149,7 @@ my ($_trunk, @_tags, @_branches, $_stdlayout);
my %icv;
my %init_opts = ( 'template=s' => \$_template, 'shared:s' => \$_shared,
'trunk|T=s' => \$_trunk, 'tags|t=s@' => \@_tags,
- 'branches|b=s@' => \@_branches, 'prefix=s' => \$_prefix,
+ 'branches|b=s@' => \@_branches, 'prefix:s' => \$_prefix,
'stdlayout|s' => \$_stdlayout,
'minimize-url|m!' => \$Git::SVN::_minimize_url,
'no-metadata' => sub { $icv{noMetadata} = 1 },
Which makes the argument to --prefix optional (in which case it is set
to "").
I'm not really sure what the best thing to do here is. I thought the
documentation talked somewhere about using --prefix="" (or just --prefix=)
to get the old behavior, but now I can't find that. In that
case I think probably just changing the tests to use --prefix ""
instead of --prefix="" should take care of it especially since the log
message for fe191fca (which actually changes the default) talks about
using --prefix "" and not --prefix="". I have attached a patch below
(against master) to make that change to the two affected subtests of t9117.
--Kyle
>8
Subject: [PATCH] t9117: use --prefix "" instead of --prefix=""
Versions of Perl's Getopt::Long module before 2.37 do not contain
this fix that first appeared in Getopt::Long version 2.37:
* Bugfix: With gnu_compat, --foo= will no longer trigger "Option
requires an argument" but return the empty string.
Instead of using --prefix="" use --prefix "" when testing an
explictly empty prefix string in order to work with older versions
of Perl's Getopt::Long module.
Signed-off-by: Kyle J. McKay
---
t/t9117-git-svn-init-clone.sh | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
index dfed76fe..a66f43c6 100755
--- a/t/t9117-git-svn-init-clone.sh
+++ b/t/t9117-git-svn-init-clone.sh
@@ -101,18 +101,18 @@ test_expect_success 'clone with -s/-T/-b/-t assumes
--prefix=origin/' '
rm -f warning
'
-test_expect_success 'init with -s/-T/-b/-t and --prefix="" still works' '
+test_expect_success 'init with -s/-T/-b/-t and --prefix "" still works' '
test ! -d project &&
- git svn init -s "$svnrepo"/project project --prefix="" 2>warning &&
+ git svn init -s "$svnrepo"/project project --prefix "" 2>warning &&
test_must_fail grep -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
rm -f warning
'
-test_expect_success 'clone with -s/-T/-b/-t and --prefix="" still works' '
+test_expect_success 'clone with -s/-T/-b/-t and --prefix "" still works' '
test ! -d project &&
- git svn clone -s "$svnrepo"/project --prefix="" 2>warning &&
+ git svn clone -s "$svnrepo"/project --prefix "" 2>warning &&
test_must_fail grep -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
--
1.8.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Project idea: strbuf allocation modes
Michael Haggerty writes: > On 04/22/2014 09:07 AM, Matthieu Moy wrote: > > The whole point of the change is to *allow* strbuf to be used in > performance-critical stuff. OK. It should not make the current use of strbuf any harder anyway. >> In your proposal, would STRBUF_OWNS_MEMORY be a constant, or a flag that >> change when the internal buffer needs reallocation? My understanding is >> that it should change (if STRBUF_FIXED_MEMORY is not set), and the >> strbuf wrapping a preallocated buffer would become a "normal" strbuf >> when its internal buffer grows. > > Correct. STRBUF_OWNS_MEMORY itself is of course a constant like 0x02 Yes, I meant "the STRBUF_OWNS_MEMORY flag". > How does the size of this project compare to what you are looking for > for your Ensimag students? I do not yet have applications for this project (it should come within the next few days). It greatly depends on students and team size. I always start with a "warm up patch" (like GSoC's microprojects), and depending on the time taken for it, I direct students to bigger or smaller projects. Your proposal is a bit tricky (it requires a good understanding of C and memory management), but it is a purely local change (i.e. you can take strbuf.[cf] out of Git's source code and hack it as a ~500 LOC project, unit-test and document the result in a self-contained way), so it should be doable. Using the new API features here and there in Git's source code is another story though. I've added the proposal with a link to this discussion here: https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#Allow_finer_memory_management_in_the_strbuf_API -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [RFC] Speed up "git status" by caching untracked file info
On Tue, Apr 22, 2014 at 5:13 PM, Duy Nguyen wrote:
>> IIRC name_hash.c::lazy_init_name_hash took ~100ms on my system, so hopefully
>> you did a dummy 'cache_name_exists("anything")' before starting the
>> measurement of the first run?
>
> No I didn't. Thanks for pointing it out. I'll see if I can reduce its time.
Well name-hash is only used when core.ignorecase is set. So it's
optional. Maybe we could save it in a separate index extension, but we
need to verify that the reader uses the same hash function as the
writer.
>> Similarly, the '--directory' option controls early returns from the
>> directory scan (via read_directory_recursive's check_only argument), so you
>> won't be able to get a full untracked files listing if the cache was
>> recorded with '--directory'. Additionally, '--directory' aggregates the
>> state at the topmost untracked directory, so that directory's cached state
>> depends on all sub-directories as well...
>
> I missed this. We could ignore check_only if caching is enabled, but
> that does not sound really good. Let me think about it more..
We could save "check_only" to the cache as well. This way we don't
have to disable the check_only trick completely.
So we process a directory with check_only set, find one untracked
entry and stop short. We store check_only value and the status ("found
something") in addition to dir mtime. Next time we check the dir's
mtime. If it matches and is called with check_only set, we know there
is at least one untracked entry, that's enough to stop r_d_r and
return early. If dir mtime does not match, or r_d_r is called without
check_only, we ignore the cached data and fall back to opendir.
Sounds good?
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] tag: add -i and --introduced modifier for --contains
On Thu 17-04-14 10:04:52, Junio C Hamano wrote: > So perhaps the rule should be updated to do something like: > > - find candidate tags that can be used to "describe --contains" > the commit A, yielding v3.4, v3.5 (not shown), and v9.0; > > - among the candidate tags, cull the ones that contain another > candidate tag, rejecting v3.5 (not shown) and v9.0; > > - among the surviving tags, pick the closest. I guess all parties agree with the first two points (and actually I would prefer not to assume anything about tag names and consider v3.4-rc1 as good as v3.4). Regarding the strategy what to select when there are several remaining tags after first two steps I would prefer to output all such tags. As people have mentioned in this thread it varies a lot between projects what people want to see (and in some cases I can imagine people really *want* to see all the tags). So printing all such tags would let them select the desired tag with grep or some more elaborate scripting... Just a thought. Honza -- Jan Kara SUSE Labs, CR -- 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: [RFC] Speed up "git status" by caching untracked file info
On Tue, Apr 22, 2014 at 4:56 PM, Karsten Blees wrote:
> Am 17.04.2014 07:51, schrieb Nguyễn Thái Ngọc Duy:
>> This patch serves as a heads up about a feature I'm working on. I hope
>> that by posting it early, people could double check if I have made
>> some fundamental mistakes that completely ruin the idea. It's about
>> speeding up "git status" by caching untracked file info in the index
>> _if_ your file system supports it (more below).
>>
>> The whole WIP series is at
>>
>> https://github.com/pclouds/git/commits/untracked-cache
>>
>> I only post the real meat here. I'm aware of a few incomplete details
>> in this patch, but nothing fundamentally wrong. So far the numbers are
>> promising. ls-files is updated to run fill_directory() twice in a
>> row and "ls-files -o --directory --no-empty-directory --exclude-standard"
>> (with gcc -O0) gives me:
>>
>>first run second (cached) run
>> gentoo-x86500 ms 71.6 ms
>> wine 140 ms 9.72 ms
>> webkit125 ms 6.88 ms
>
> IIRC name_hash.c::lazy_init_name_hash took ~100ms on my system, so hopefully
> you did a dummy 'cache_name_exists("anything")' before starting the
> measurement of the first run?
No I didn't. Thanks for pointing it out. I'll see if I can reduce its time.
>> The following inputs are sufficient to determine what files in a
>> directory are excluded:
>>
>> - The list of files and directories of the direction in question
>> - The $GIT_DIR/index
>> - The content of $GIT_DIR/info/exclude
>> - The content of core.excludesfile
>> - The content (or the lack) of .gitignore of all parent directories
>>from $GIT_WORK_TREE
>>
>
> The dir_struct.flags also play a big role in evaluation of read_directory.
>
> E.g. it seems untracked files are not properly recorded if the cache is
> filled with '--ignored' option:
Yeah. dir_struct.flags will be part of the input. I intend to optimize
"git status" case only, so if it matches the recorded
dir_struct.flags, the cache is used. Else the cache is ignored.
Caching --ignored is not so interesting, because the list of ignored
files could be huge, while untracked listing is usually small.
>> @@ -1360,15 +1603,18 @@ static enum path_treatment
>> read_directory_recursive(struct dir_struct *dir,
>> break;
>>
>> case path_untracked:
>> - if (!(dir->flags & DIR_SHOW_IGNORED))
>> - dir_add_name(dir, path.buf, path.len);
>> + if (dir->flags & DIR_SHOW_IGNORED)
>> + break;
>> + dir_add_name(dir, path.buf, path.len);
>> + if (cdir.fdir)
>> + add_untracked(untracked, path.buf + baselen);
>> break;
>
> Similarly, the '--directory' option controls early returns from the directory
> scan (via read_directory_recursive's check_only argument), so you won't be
> able to get a full untracked files listing if the cache was recorded with
> '--directory'. Additionally, '--directory' aggregates the state at the
> topmost untracked directory, so that directory's cached state depends on all
> sub-directories as well...
I missed this. We could ignore check_only if caching is enabled, but
that does not sound really good. Let me think about it more..
>
> I wonder if it makes sense to separate cache recording logic from
> read_directory_recursive and friends, which are mainly concerned with flags
> processing.
The core code path is still shared though, or we would duplicate r_d_r
entirely for caching recording, which sounds like a maintenance
nightmare.
>> At the implementation level, the whole directory structure is saved,
>> each directory corresponds to one struct untracked_dir.
>
> With the usual options (e.g. standard 'git status'), untracked directories
> are mostly skipped, so the cache would mostly store tracked directories.
> Naming it 'struct untracked_dir' is a bit confusing, IMO.
It's actually just "directories". We may need to store both tracked
and untracked directories. Maybe renaming it to cached_dir..
>> So if all is really well, read_directory() becomes a series of
>> open(".gitignore"), read(".gitignore"), close(), hash_sha1_file() and
>> stat() _without_ heavyweight exclude filtering. There should be
>> no overhead if this feature is disabled.
>>
>
> Wouldn't mtime of .gitignore files suffice here (so you don't need to open
> and parse them every time)?
That's a further optimization. With the current code it's simpler to
open .gitignore. Assume you have a path a/b/c. a/.gitignore's stat
info is good, so you skip opening it. Then you find a/b/.gitignore is
modified and you need to recompute untracked files in a/b. To do that
you need a/.gitignore as well. Lazily opening a/.gitignore at this
stage is possible, but trickier (you have to make sure the rules are
in correct order because of negative patterns).
A
Re: [PATCH v2 2/2] blame: use different blame_date_width for different locale
Junio C Hamano writes:
> This is not wrong per-se, but I am not sure if translators would
> understand that "years and months ago" may not be the longuest
> variant for their language and they are asked to use "89 seconds
> ago" if the translation of that is longer than the translation for
> "4 years, 11 months ago" in their language, with the given
> explanation.
What's with the 89? And the other semi-magic numbers? If we fear about
non-arabic number formatting, at least in French French the worst
offender may be quatre-vingt-quatorze ("four score and fourteen") or
quatre-vingt-dix-neuf ("four score and nineteen"), namely 94 or 99. But
I think it's improbable to get worded formatting here anyway. Or are
those the largest values with their respective granularity?
--
David Kastrup
--
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: Project idea: github-like diff view
On Sun, Apr 20, 2014 at 9:46 PM, Jeff King wrote: > On Sun, Apr 20, 2014 at 04:58:28PM +0700, Duy Nguyen wrote: > >> - --color-words within unified diff format, using background color to >> show what part of the line has changed. This is only enabled for >> 1-line changes. > > See contrib/diff-highlight. Thanks. I'd rather have it built in core git still. I'll try to see if I can rewrite it in C. Else, any objection to promote it to a core helper and setup pager automatically? We can have a config key to turn it off, but if git diff is colored, then it could be on by default. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Speed up "git status" by caching untracked file info
Am 17.04.2014 07:51, schrieb Nguyễn Thái Ngọc Duy:
> This patch serves as a heads up about a feature I'm working on. I hope
> that by posting it early, people could double check if I have made
> some fundamental mistakes that completely ruin the idea. It's about
> speeding up "git status" by caching untracked file info in the index
> _if_ your file system supports it (more below).
>
> The whole WIP series is at
>
> https://github.com/pclouds/git/commits/untracked-cache
>
> I only post the real meat here. I'm aware of a few incomplete details
> in this patch, but nothing fundamentally wrong. So far the numbers are
> promising. ls-files is updated to run fill_directory() twice in a
> row and "ls-files -o --directory --no-empty-directory --exclude-standard"
> (with gcc -O0) gives me:
>
>first run second (cached) run
> gentoo-x86500 ms 71.6 ms
> wine 140 ms 9.72 ms
> webkit125 ms 6.88 ms
IIRC name_hash.c::lazy_init_name_hash took ~100ms on my system, so hopefully
you did a dummy 'cache_name_exists("anything")' before starting the measurement
of the first run?
> linux-2.6 106 ms 16.2 ms
>
> Basically untracked time is cut to one tenth in the best case
> scenario. The final numbers would be a bit higher because I haven't
> stored or read the cache from index yet. Real commit message follows..
>
>
> read_directory() plays a bit part in the slowness of "git status"
> because it has to read every directory and check for excluded entries,
> which is really expensive. This patch adds an option to cache the
> results so that after the first slow read_directory(), the following
> calls should be cheap and fast.
>
> The following inputs are sufficient to determine what files in a
> directory are excluded:
>
> - The list of files and directories of the direction in question
> - The $GIT_DIR/index
> - The content of $GIT_DIR/info/exclude
> - The content of core.excludesfile
> - The content (or the lack) of .gitignore of all parent directories
>from $GIT_WORK_TREE
>
The dir_struct.flags also play a big role in evaluation of read_directory.
E.g. it seems untracked files are not properly recorded if the cache is filled
with '--ignored' option:
> @@ -1360,15 +1603,18 @@ static enum path_treatment
> read_directory_recursive(struct dir_struct *dir,
> break;
>
> case path_untracked:
> - if (!(dir->flags & DIR_SHOW_IGNORED))
> - dir_add_name(dir, path.buf, path.len);
> + if (dir->flags & DIR_SHOW_IGNORED)
> + break;
> + dir_add_name(dir, path.buf, path.len);
> + if (cdir.fdir)
> + add_untracked(untracked, path.buf + baselen);
> break;
Similarly, the '--directory' option controls early returns from the directory
scan (via read_directory_recursive's check_only argument), so you won't be able
to get a full untracked files listing if the cache was recorded with
'--directory'. Additionally, '--directory' aggregates the state at the topmost
untracked directory, so that directory's cached state depends on all
sub-directories as well...
I wonder if it makes sense to separate cache recording logic from
read_directory_recursive and friends, which are mainly concerned with flags
processing.
> If we can cheaply validate all those inputs for a certain directory,
> we are sure that the current code will always produce the same
> results, so we can cache and reuse those results.
>
> This is not a silver bullet approach. When you compile a C file, for
> example, the old .o file is removed and a new one with the same name
> created, effectively invalidating the containing directory's
> cache. But at least with a large enough work tree, there could be many
> directories you never touch. The cache could help there.
>
> The first input can be checked using directory mtime. In many
> filesystems, directory mtime is updated when direct files/dirs are
> added or removed (*). If you do not use such a file system, this
> feature is not for you.
>
> The second one can be hooked from read-cache.c. Whenever a file (or a
> submodule) is added or removed from a directory, we invalidate that
> directory. This will be done in a later patch.
>
> The remaining inputs are easy, their SHA-1 could be used to verify
> their contents. We do need to read .gitignore files and digest
> them. But they are usually few and small, so the overhead should not
> be much.
>
> At the implementation level, the whole directory structure is saved,
> each directory corresponds to one struct untracked_dir.
With the usual options (e.g. standard 'git status'), untracked directories are
mostly skipped, so the cache would mostly store tracked directories. Naming it
'struct untracked_dir' is a bit confusing, IMO.
> Each directory
> ho
Re: Fwd: git p4: feature request - branch check filtering
Hi Pete,
I should have updated on this earlier, but I wished to refine my work
on this feature before submitting. With 2.0 looming I'll submit
what's there so far.
There is a patch viewable at this link:
https://github.com/Stealthii/git/commit/f7a2e611262fd977ac99e066872d3d0743b7df3c
For the use case this works perfectly - if I define branch mappings
with git config, followed by setting 'git-p4.skipBranchScan' to true,
git-p4 will skip scanning of all remote branches and limit to what's
defined in the map. An example config:
[git-p4]
skipBranchScan = true
branchList = release_1.0.0:release_1.1.0
branchList = release_1.1.0:release_1.2.0
If there is any more information I need to provide let me know. I have
been using this patch for over two months, testing both use cases with
and without git-p4.skipBranchScan and I have noticed no issues. Logic
of git-p4 is not changed from default behaviour, unless the user
explicitly sets the boolean flag to skip scanning.
Dan
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager. This message contains confidential information and
is intended only for the individual named. If you are not the named
addressee you should not disseminate, distribute or copy this e-mail.
Please notify the sender immediately by e-mail if you have received
this e-mail by mistake and delete this e-mail from your system. If you
are not the intended recipient you are notified that disclosing,
copying, distributing or taking any action in reliance on the contents
of this information is strictly prohibited.
On 23 February 2014 15:12, Pete Wyckoff wrote:
> dpr...@gmail.com wrote on Tue, 18 Feb 2014 12:42 +:
>> I work at a company that has recently moved all CVS, SVN, and git
>> repositories to Perforce. Depots have not been setup correctly in
>> every case, and there is one depot that contains literally hundreds of
>> projects under commercial development (and hundreds of branches as a
>> result)
>
> My condolences.
>
>> My project may be in //stupid_depot/commercial/teamporter/rok. This
>> is the path I clone with git-p4. The only branches in this depot that
>> contain files at this path are titled as
>> 'rok_porter_branch/release_1.x' or similar.
>>
>> When using '--detect-branches' git-p4 checks each key of branches to
>> see if any of them have files in the path I've cloned. Whilst this is
>> good in practice there is unfortunately 6,809 branches, git-p4
>> processes about 2 a second and just under an hour to perform any
>> git-p4 rebase, submit, or similar operation.
>
> This is in getBranchMapping() presumably. Where it loops
> over each branch doing "p4 branch -o". Yuk.
>
> You could always avoid the --detect-branches if you don't really
> need it, instead doing, say, multiple "git p4 sync" for the
> different areas of the repo that interest you, each with its own
> destination branch in git ("p4/depot-part1", "p4/depot-part3",
> ...). Or --use-client-spec to cobble together an exact mapping
> of where p4 files should land in git, all in a single git branch
> then.
>
>> I propose the addition of a branch list filtering option
>> (--filter-branches) that takes either a regular expression or list of
>> branches it should check. This may be useful in sane situations where
>> you don't want to scan every branch in a Perforce repository, or
>> blacklist branches that have undesirable content (for example, one of
>> the branches is called 'svn-backup'. It contains a single, multi-GB
>> tarball.)
>
> There is the existing git-p4.branchList option that explicitly
> adds (or overrides) branch information, beyond the ones auto-discovered.
>
> You might be able to use that option, but change its behavior
> to avoid the scan. So that if that option is set in the config,
> p4 is not asked anything about its branches. Not sure if this
> would break anyone's setup though.
>
> Another approach would be to add a config option
> git-p4.branchScan that defaults to True. You could turn it off
> and use branchList.
>
>> It would be ideal to have this information (after initial clone or
>> sync) stored somewhere in the git config where is appropriate so that
>> future submit/rebase operations adhere to this list.
>>
>> Has something like this been worked on, or has been considered in the
>> past? If not I will consider implementing this after reading up on
>> the Git code guidelines.
>>
>> Thanks for keeping the Git workflow accessible in painful areas.
>
> It would be great if you could get something like this to work.
> Start in getBranchMapping() and don't forget to write up your
> work in Documentation/git-p4.txt. Also, this is sort of a messy
> area of the code, unfortunately. t/t9801 tries to make sure some
> of it keeps working.
>
> -- Pete
>
--
To unsubscribe from this
git p4: bug - branch detection broken on empty branches
As part of my work to help get git-p4 close to bug-free before Git
2.0, I'm posting all bugs and patches to this mailing list. Please
direct me elsewhere if this is incorrect.
When trying to clone a particular directory from a depot, that
contains one or more branches that contain no commits for that
directory, branch detection is broken and results in a failed clone.
fatal: ambiguous argument
'refs/remotes/p4/silly_project_branch/trunk': unknown revision or path
not in the working tree.
Use '--' to separate paths from revisions, like this:
'git [...] -- [...]'
Traceback (most recent call last):
File "/home/dreid/bin/git-p4", line 3295, in ?
main()
File "/home/dreid/bin/git-p4", line 3289, in main
if not cmd.run(args):
File "/home/dreid/bin/git-p4", line 3163, in run
if not P4Sync.run(self, depotPaths):
File "/home/dreid/bin/git-p4", line 3016, in run
self.importChanges(changes)
File "/home/dreid/bin/git-p4", line 2678, in importChanges
blob = self.searchParent(parent, branch, tempBranch)
File "/home/dreid/bin/git-p4", line 2600, in searchParent
for blob in read_pipe_lines(["git", "rev-list", "--reverse",
File "/home/dreid/bin/git-p4", line 155, in read_pipe_lines
die('Command failed: %s' % str(c))
File "/home/dreid/bin/git-p4", line 106, in die
raise Exception(msg)
Exception: Command failed: ['git', 'rev-list', '--reverse',
'--no-merges', 'refs/remotes/p4/silly_project_branch/trunk']
Original command:
$ git-p4 clone
//insane_depot/projects/Exchange/CompanyName/silly_project_branch@all
silly-project --detect-branches -v
--
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: Project idea: strbuf allocation modes
On 04/22/2014 09:07 AM, Matthieu Moy wrote: > Michael Haggerty writes: > >> STRBUF_OWNS_MEMORY >> >> The memory pointed to by buf is owned by this strbuf. If this >> bit is not set, then the memory should never be freed, and >> (among other things) strbuf_detach() must always call xstrcpy(). > > I just foresee a small difficulty from the caller side: > > char path_buf[PATH_MAX]; > char *detached; > > struct strbuf path; > > strbuf_wrap_preallocated(&path, path_buf, 0, sizeof(path)); > ... > detached = strbuf_detach(&path); > > the wrapping/unwrapping of path_buf magically turned a stack-allocated > buffer into a heap-allocated one. And the initial goal of avoiding > malloc() is defeated. So, essentially, one should avoir using > strbuf_wrap_preallocated with strbuf_detach, right? Correct; it wouldn't bring any performance advantages. But strbuf_detach() should be implemented for such strbufs for completeness so that after the strbuf is instantiated, users don't have to know its allocation mode. > But I agree with > Junio that if the API is properly used, everything should work. I'm just > worried that we will add a bit more complexity to the API, and I'm not > sure we can actually expect noticeable improvements in terms of > performance (i.e. do we actually use strbuf for performance-critical > stuff?). The whole point of the change is to *allow* strbuf to be used in performance-critical stuff. > In your proposal, would STRBUF_OWNS_MEMORY be a constant, or a flag that > change when the internal buffer needs reallocation? My understanding is > that it should change (if STRBUF_FIXED_MEMORY is not set), and the > strbuf wrapping a preallocated buffer would become a "normal" strbuf > when its internal buffer grows. Correct. STRBUF_OWNS_MEMORY itself is of course a constant like 0x02 (a mask to set/clear a bit in the flags) but the corresponding flags bit would sometimes be changed internal to the strbuf implementation. For example, if the buffer is grown past its original size (when STRBUF_FIXED_MEMORY is not set) the bit would be set. After strbuf_detach(), there would be no automatic way to re-attach such a strbuf to its original heap-allocated memory. So in this situation the buffer would have to be pointed at strbuf_slopbuf again, and the STRBUF_OWNS_MEMORY bit would be cleared again. > If so, your "strbuf_detach() must always call xstrcpy()" is to be > understood as "if STRBUF_OWNS_MEMORY is still set when strbuf_detach() > is called, then it must always call xstrcpy()", right? Exactly. How does the size of this project compare to what you are looking for for your Ensimag students? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.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: [SECURITY PATCH] git-prompt.sh: don't put unsanitized branch names in $PS1
On 04/21/2014 10:24 PM, Jeff King wrote:
> On Mon, Apr 21, 2014 at 03:07:28PM -0400, Richard Hansen wrote:
>
>> Both bash and zsh subject the value of PS1 to parameter expansion,
>> command substitution, and arithmetic expansion. Rather than include
>> the raw, unescaped branch name in PS1 when running in two- or
>> three-argument mode, construct PS1 to reference a variable that holds
>> the branch name. Because the shells do not recursively expand, this
>> avoids arbitrary code execution by specially-crafted branch names such
>> as '$(IFS=_;cmd=sudo_rm_-rf_/;$cmd)'.
>
> Cute. We already disallow quite a few characters in refnames (including
> space, as you probably discovered), and generally enforce that during
> ref transfer. I wonder if we should tighten that more as a precuation.
> It would be backwards-incompatible, but I wonder if things like "$" and
> ";" in refnames are actually useful to people.
While we're at it, I think it would be prudent to ban '-' at the
beginning of reference name segments. For example, reference names like
refs/heads/--cmd=/sbin/halt
refs/tags/--exec=forkbomb(){forkbomb|forkbomb&};forkbomb
are currently both legal, but I think they shouldn't be. I wouldn't be
surprised if somebody could find a way to exploit
references-named-like-command-line-options.
At a minimum, it is very difficult to write scripts robust against such
names. Some branch- and tag-oriented commands *require* short names and
don't allow the full reference name including refs/heads/ or refs/tags/
to be specified. In such cases there is no systematic way to prevent
the names from being seen as command-line options. And '--' by itself,
which many Unix commands use to separate options from arguments, has a
different meaning in Gitland.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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: What is missing from Git v2.0
Sebastian Schuberth wrote: > On Mon, Apr 21, 2014 at 10:46 PM, Felipe Contreras > wrote: > > >> The problem is that between "git rm" and "git mv", if we default "git > >> cp" to mean "cherry-pick" there could easily be user confusion. > >> > >> I'm not sure that cherry-pick is used that often it really needs a two > >> character shortcut. Maybe just "git pick"? > > > > I'm in favor of having both 'git pick', and 'git pi'. > > Please let's stick to one clear default alias for some central > commands, and not introduce ambiguity between aliases. That said, I'd > prefer pick over pi, but still cp would be my personal favorite. I think porcelain commands should eventually change names (and maybe become more friendly) 'git ls-files' => 'git ls', 'git cherry-pick' => 'git pick', and so on. If 'cherry-pick' evolves into 'pick', it would make sense to have the 'pi' alias, and in preparation for that I don't see why a temporary 'pick' alias would hurt. -- Felipe Contreras -- 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 is missing from Git v2.0
Sebastian Schuberth wrote: > On Mon, Apr 21, 2014 at 9:39 PM, Junio C Hamano wrote: > > >> If we don't standardize this now people will come up with their own > >> definitions [1] [2] (and many others if you just search GitHub) which > >> are again likely to differ (slightly), hindering interoperability. > > > > I am afraid that that ship has sailed long time ago, though. > > That's most likely true, but it does not get better by postponing this > even more. I still think there's value in introducing this now, Git > still attracts new developers every day. In fact, I currently see a > leap forwarding in the Windows world towards Git, caused by some > rethinking and structural changes in some big companies. Exactly. If one thinks in terms of years, sure, it might make sense to not change the status quo created by years back. But think about Git in a decade or two, at that point surely you would have wished that you had considered these kinds of changes sooner. Junio at some point suggested to think about features for v1.8.0 as if we were starting from scratch[1]. I'd say if there has every been a time to add default aliases after v1.0 it's certainly v2.0. Our future users who might have not touched Git yet would certainly welcome this. [1] http://article.gmane.org/gmane.comp.version-control.git/165735 -- Felipe Contreras -- 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 2/3] test-lib: tests skipped by GIT_SKIP_TESTS say so
We used to show "(missing )" next to tests skipped because they are
specified in GIT_SKIP_TESTS. Use "(GIT_SKIP_TESTS)" instead.
Plus tests that check basic GIT_SKIP_TESTS functions.
Signed-off-by: Ilya Bobyr
---
t/t-basic.sh | 63 ++
t/test-lib.sh| 13 ++
2 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/t/t-basic.sh b/t/t-basic.sh
index a2bb63c..ae8874e 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -270,6 +270,69 @@ test_expect_success 'test --verbose-only' '
EOF
'
+test_expect_success 'GIT_SKIP_TESTS' "
+ GIT_SKIP_TESTS='git.2' \
+ run_sub_test_lib_test git-skip-tests-basic \
+ 'GIT_SKIP_TESTS' <<-\\EOF &&
+ for i in 1 2 3
+ do
+ test_expect_success \"passing test #\$i\" 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test git-skip-tests-basic <<-\\EOF
+ > ok 1 - passing test #1
+ > ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+ > ok 3 - passing test #3
+ > # passed all 3 test(s)
+ > 1..3
+ EOF
+"
+
+test_expect_success 'GIT_SKIP_TESTS several tests' "
+ GIT_SKIP_TESTS='git.2 git.5' \
+ run_sub_test_lib_test git-skip-tests-several \
+ 'GIT_SKIP_TESTS several tests' <<-\\EOF &&
+ for i in 1 2 3 4 5 6
+ do
+ test_expect_success \"passing test #\$i\" 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test git-skip-tests-several <<-\\EOF
+ > ok 1 - passing test #1
+ > ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+ > ok 3 - passing test #3
+ > ok 4 - passing test #4
+ > ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+ > ok 6 - passing test #6
+ > # passed all 6 test(s)
+ > 1..6
+ EOF
+"
+
+test_expect_success 'GIT_SKIP_TESTS sh pattern' "
+ GIT_SKIP_TESTS='git.[2-5]' \
+ run_sub_test_lib_test git-skip-tests-sh-pattern \
+ 'GIT_SKIP_TESTS sh pattern' <<-\\EOF &&
+ for i in 1 2 3 4 5 6
+ do
+ test_expect_success \"passing test #\$i\" 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test git-skip-tests-sh-pattern <<-\\EOF
+ > ok 1 - passing test #1
+ > ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
+ > ok 3 # skip passing test #3 (GIT_SKIP_TESTS)
+ > ok 4 # skip passing test #4 (GIT_SKIP_TESTS)
+ > ok 5 # skip passing test #5 (GIT_SKIP_TESTS)
+ > ok 6 - passing test #6
+ > # passed all 6 test(s)
+ > 1..6
+ EOF
+"
+
test_set_prereq HAVEIT
haveit=no
test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c081668..e7d9c51 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -452,25 +452,28 @@ test_finish_ () {
test_skip () {
to_skip=
+ skipped_reason=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
to_skip=t
+ skipped_reason="GIT_SKIP_TESTS"
fi
if test -z "$to_skip" && test -n "$test_prereq" &&
! test_have_prereq "$test_prereq"
then
to_skip=t
- fi
- case "$to_skip" in
- t)
+
of_prereq=
if test "$missing_prereq" != "$test_prereq"
then
of_prereq=" of $test_prereq"
fi
-
+ skipped_reason="missing $missing_prereq${of_prereq}"
+ fi
+ case "$to_skip" in
+ t)
say_color skip >&3 "skipping test: $@"
- say_color skip "ok $test_count # skip $1 (missing
$missing_prereq${of_prereq})"
+ say_color skip "ok $test_count # skip $1 ($skipped_reason)"
: true
;;
*)
--
1.7.9
--
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 1/3] test-lib: Document short options in t/README
Most arguments that could be provided to a test have short forms. Unless documented, the only way to learn them is to read the code. Signed-off-by: Ilya Bobyr --- t/README |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/t/README b/t/README index caeeb9d..6b93aca 100644 --- a/t/README +++ b/t/README @@ -71,7 +71,7 @@ You can pass --verbose (or -v), --debug (or -d), and --immediate (or -i) command line argument to the test, or by setting GIT_TEST_OPTS appropriately before running "make". ---verbose:: +-v,--verbose:: This makes the test more verbose. Specifically, the command being run and their output if any are also output. @@ -81,7 +81,7 @@ appropriately before running "make". numbers matching . The number matched against is simply the running count of the test within the file. ---debug:: +-d,--debug:: This may help the person who is developing a new test. It causes the command defined with test_debug to run. The "trash" directory (used to store all temporary data @@ -89,14 +89,14 @@ appropriately before running "make". failed tests so that you can inspect its contents after the test finished. ---immediate:: +-i,--immediate:: This causes the test to immediately exit upon the first failed test. Cleanup commands requested with test_when_finished are not executed if the test failed, in order to keep the state for inspection by the tester to diagnose the bug. ---long-tests:: +-l,--long-tests:: This causes additional long-running tests to be run (where available), for more exhaustive testing. -- 1.7.9 -- 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 3/3] test-lib: '--run' to run only specific tests
Allow better control of the set of tests that will be executed for a
single test suite. Mostly useful while debugging or developing as it
allows to focus on a specific test.
Signed-off-by: Ilya Bobyr
---
t/README | 73 +++-
t/t-basic.sh | 356 +-
t/test-lib.sh| 109 +
3 files changed, 530 insertions(+), 8 deletions(-)
diff --git a/t/README b/t/README
index 6b93aca..2dac619 100644
--- a/t/README
+++ b/t/README
@@ -100,6 +100,11 @@ appropriately before running "make".
This causes additional long-running tests to be run (where
available), for more exhaustive testing.
+-r,--run=::
+ Run only the subset of tests indicated by
+ . See section "Skipping Tests" below for
+syntax.
+
--valgrind=::
Execute all Git binaries under valgrind tool and exit
with status 126 on errors (just like regular tests, this will
@@ -187,10 +192,70 @@ and either can match the "t[0-9]{4}" part to skip the
whole
test, or t[0-9]{4} followed by ".$number" to say which
particular test to skip.
-Note that some tests in the existing test suite rely on previous
-test item, so you cannot arbitrarily disable one and expect the
-remainder of test to check what the test originally was intended
-to check.
+For an individual test suite --run could be used to specify that
+only some tests should be run or that some tests should be
+excluded from a run.
+
+The argument for --run is a list of individual test numbers or
+ranges with an optional negation prefix that define what tests in
+a test suite to include in the run. A range is two numbers
+separated with a dash and matches a range of tests with both ends
+been included. You may omit the first or the second number to
+mean "from the first test" or "up to the very last test"
+respectively.
+
+Optional prefix of '!' means that the test or a range of tests
+should be excluded from the run.
+
+If --run starts with an unprefixed number or range the initial
+set of tests to run is empty. If the first item starts with '!'
+all the tests are added to the initial set. After initial set is
+determined every test number or range is added or excluded from
+the set one by one, from left to right.
+
+Individual numbers or ranges could be separated either by a space
+or a comma.
+
+For example, common case is to run several setup tests (1, 2, 3)
+and then a specific test (21) that relies on that setup:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
+
+or:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
+
+or:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
+
+To run only tests up to a specific test (21), one could do this:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
+
+or this:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='-21'
+
+As noted above, the test set is built going though items left to
+right, so this:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
+
+will run tests 1, 2, and 4.
+
+You may use negation with ranges. The following will run all
+test as a test suite except from 7 upto 11:
+
+$ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
+
+Some tests in a test suite rely on the previous tests performing
+certain actions, specifically some tests are designated as
+"setup" test, so you cannot _arbitrarily_ disable one test and
+expect the rest to function correctly.
+--run is mostly useful when you want to focus on a specific test
+and know what you are doing. Or when you want to run up to a
+certain test.
Naming Tests
diff --git a/t/t-basic.sh b/t/t-basic.sh
index ae8874e..e2589cc 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -42,9 +42,9 @@ test_expect_success 'success is reported like this' '
:
'
-run_sub_test_lib_test () {
- name="$1" descr="$2" # stdin is the body of the test code
- shift 2
+_run_sub_test_lib_test_common () {
+ neg="$1" name="$2" descr="$3" # stdin is the body of the test code
+ shift 3
mkdir "$name" &&
(
# Pretend we're not running under a test harness, whether we
@@ -70,10 +70,23 @@ run_sub_test_lib_test () {
export TEST_DIRECTORY &&
TEST_OUTPUT_DIRECTORY=$(pwd) &&
export TEST_OUTPUT_DIRECTORY &&
- ./"$name.sh" "$@" >out 2>err
+ if test -z "$neg"
+ then
+ ./"$name.sh" "$@" >out 2>err
+ else
+ ! ./"$name.sh" "$@" >out 2>err
+ fi
)
}
+run_sub_test_lib_test () {
+ _run_sub_test_lib_test_common '' "$@"
+}
+
+run_sub_test_lib_test_err () {
+ _run_sub_test_lib_test_common '!' "$@"
+}
+
check_sub_test_lib_test () {
name="$1" # stdin is the expected output from the test
(
@@ -84,6 +97,18 @@ check_sub_test_lib_test () {
)
}
+check_sub_test_lib_test_err () {
+
[RFC/PATCH v3] Better control of the tests run by a test suite
This patches add `--run` option to the test suites to allow one to run individual tests out of the test suite. Like this: ./t-basic.sh --run='-4,7,9-12,15-' Both spaces and commas are accepted as separators for the ranges (In previous versions only spaces were accepted). Two previous versions are here: [RFC/PATCH] Better control of the tests run by a test suite http://www.mail-archive.com/git@vger.kernel.org/msg46419.html [RFC/PATCH v2] Better control of the tests run by a test suite http://www.mail-archive.com/git@vger.kernel.org/msg46877.html In this version I have removed mathematical operators and used ranges as suggested by Junio[1] and Eric Sunshine[2]. [1] http://www.mail-archive.com/git@vger.kernel.org/msg47098.html [2] http://www.mail-archive.com/git@vger.kernel.org/msg46960.html This version also includes changes according to the comments from Eric Sunshine in the documentation. But as this version has slightly different documentation, it would be nice if someone would read it once again :) Shell patterns are not allowed any more. I think they are not that useful and ranges cover almost the same functionality. Also with patterns like '[8-9]', it is harder to produce good error messages for invalid range ends. This conversion is a bit unfinished: On 3/31/2014 10:09 AM, Junio C Hamano wrote: > I would have to say that there is already an established pattern to > pick ranges that normal people understand well and it would be silly > to invent another more verbose way to express the same thing. You > tell your Print Dialog which page to print with e.g. "-4,7,9-12,15-", > not ">=4 7 ...". > > Would the same notation be insufficient for our purpose? You do not > even have to worry about negation that way. http://www.mail-archive.com/git@vger.kernel.org/msg47098.html Negation was not necessary for my use cases even in the first version. I've added it more because it seemed to be very close to the functionality I was adding and not that complicated. So, I've left the negation in the new version as well. I am actually thinking now that --verbose-only= and --valgrind= could be switched to use the same syntax as in --run. I also noticed that I am doing the following quite often: ./t-basic.sh --run=1-4,27 --verbose-only=27 Maybe it would be better to support 'v' suffix as a flag to indicate what a test needs to be run in verbose mode: ./t-basic.sh --run=1-4,27v Ilya Bobyr (3): test-lib: Document short options in t/README test-lib: tests skipped by GIT_SKIP_TESTS say so test-lib: '--run' to run only specific tests t/README | 81 ++- t/t-basic.sh | 419 +- t/test-lib.sh| 120 +++- 3 files changed, 604 insertions(+), 16 deletions(-) -- 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
Pack bitmaps on Git for Windows
It looks like the documentation for bitmaps is being included in the 1.9.2 release of Git for Windows but the functionality itself is not present. For example, doc\git\html\git-config.html includes settings like these: pack.useBitmaps When true, git will use pack bitmaps (if available) when packing to stdout (e.g., during the server side of a fetch). Defaults to true. You should not generally need to turn this off unless you are debugging pack bitmaps. pack.writebitmaps When true, git will write a bitmap index when packing all objects to disk (e.g., when git repack -a is run). This index can speed up the "counting objects" phase of subsequent packs created for clones and fetches, at the cost of some disk space and extra time spent on the initial repack. Defaults to false. Repacking repositories with pack.writebitmaps set to true doesn't write a bitmap, suggesting the functionality is not actually present. This does not appear to be an issue with "normal" git, per the documentation for git config at http://www.git-scm.com/docs/git-config. Bryan Turner -- 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 2/2] mergetool: run prompt only if guessed tool
Charles Bailey wrote: > On Tue, Apr 22, 2014 at 01:53:46AM -0500, Felipe Contreras wrote: > > Charles Bailey wrote: > > > On Tue, Apr 22, 2014 at 01:24:09AM -0500, Felipe Contreras wrote: > > > > > > > > This is what I get when a tool is not working: > > > > > > > > Documentation/config.txt seems unchanged. > > > > Was the merge successful? [y/n] > > > > > > Does this happen now even with merge tools for which we do trust the > > > exit code? If so, my original concern is addressed. > > > > Which tools are those? > > I didn't remember off hand, but checking the mergetools directory > suggests that kdiff3 is one (there's no call to check_unchanged). I > stopped checking after I found one. So: % cat > ~/bin/kdiff3 <<-\EOF #!/bin/sh false EOF % chmod +x ~/bin/kdiff3 % git -c merge.tool=kdiff3 mergetool merge of Documentation/config.txt failed Continue merging other unresolved paths (y/n) ? > > You don't see anything wrong with asking the user *every single time* he > > runs > > `git mergetool`, even though he *already told us* which tool to use? > > > > If so, I'm pretty sure everybody else disagrees with you. > > I think that you may have misunderstood me. As I said, I've no > particular objections to changing the default (subject a few concerns > that may or may not still apply). > > Having said that, the fact that the user has configured the merge tool > doesn't mean that he necessarily doesn't want to see the prompt. Not necessarily, but in 99% of the cases it does. And for the remaining there's always mergetool.prompt = true. > In a part of my reply which you snipped, I said that it's sometimes the > particular file that's due to be resolved that might prompt a user to want to > skip launching the tool. That's a possibility, however, in almost all the situations I've wanted to stop a merge, it's *after* I've seen the actual conflicts in the file. Anyway, I've revisited the code, and it's only now that I've realized that this: Hit return to start merge resolution tool (kdiff3): Is not actually asking me for the tool I want to use; the value in parenthesis is not the default, and I can't type in another tool. So the purpose of the prompt is very different from what I was thinking, yet I still think the value of such prompt is marginal at best. > Either way, I think we shouldn't unconditionally override an explicitly > set mergetool.prompt and if we are (effectively) changing the default we > should probably update the documentation to say so as well. An explicitly set mergetool.prompt = true would override the default. See the patch. I looked, the documentation doesn't mention any default. We could add it, but I don't think it's necesarily part of this patch. -- Felipe Contreras -- 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 2/2] mergetool: run prompt only if guessed tool
On Tue, Apr 22, 2014 at 01:53:46AM -0500, Felipe Contreras wrote: > Charles Bailey wrote: > > On Tue, Apr 22, 2014 at 01:24:09AM -0500, Felipe Contreras wrote: > > > > > > This is what I get when a tool is not working: > > > > > > Documentation/config.txt seems unchanged. > > > Was the merge successful? [y/n] > > > > Does this happen now even with merge tools for which we do trust the > > exit code? If so, my original concern is addressed. > > Which tools are those? > I didn't remember off hand, but checking the mergetools directory suggests that kdiff3 is one (there's no call to check_unchanged). I stopped checking after I found one. > You don't see anything wrong with asking the user *every single time* he runs > `git mergetool`, even though he *already told us* which tool to use? > > If so, I'm pretty sure everybody else disagrees with you. I think that you may have misunderstood me. As I said, I've no particular objections to changing the default (subject a few concerns that may or may not still apply). Having said that, the fact that the user has configured the merge tool doesn't mean that he necessarily doesn't want to see the prompt. In a part of my reply which you snipped, I said that it's sometimes the particular file that's due to be resolved that might prompt a user to want to skip launching the tool. Either way, I think we shouldn't unconditionally override an explicitly set mergetool.prompt and if we are (effectively) changing the default we should probably update the documentation to say so as well. (Yes, I didn't introduce the first "no prompt" option patch and yes, I have since changed my mind about whether I have it set, but that's just a personal preference.) -- 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
