git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Topics currently in the Stalled category
  2012-11-20 23:50 ` Junio C Hamano
@ 2012-11-21  0:05   ` Junio C Hamano
  2012-11-21  3:31     ` Felipe Contreras
                       ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Junio C Hamano @ 2012-11-21  0:05 UTC (permalink / raw)
  To: git

Here is a list of stalled topics I am having trouble deciding what
to do (the default is to dismiss them around feature freeze).

* fc/fast-export-fixes (2012-11-08) 14 commits

 Renaming of remote-testgit feels to be a mistake.  It probably
 should keep its source in remote-testgit.bash and generate it, and
 moreover, if it wants to rename remote-testgit.py to remote-testpy,
 the new one should be called remote-testbash.  There was one reroll
 after what used to be queued, but nobody seemed to be interested in
 reviewing the series.

 This mostly happened while I was away, and judging from the
 discussion around this topic (and earlier iterations), I do not
 feel comfortable merging this series (or v5 reroll) as-is.

 Help?


* pf/editor-ignore-sigint (2012-11-11) 5 commits

 Avoid confusing cases where the user hits Ctrl-C while in the editor
 session, not realizing git will receive the signal. Since most editors
 will take over the terminal and will block SIGINT, this is not likely
 to confuse anyone.

 Some people raised issues with emacsclient, which are addressed by this
 re-roll. It should probably also handle SIGQUIT, and there were a
 handful of other review comments.

 Anybody interested in moving this forward?
 

* mo/cvs-server-updates (2012-10-16) 10 commits
 - cvsserver Documentation: new cvs ... -r support
 - cvsserver: add t9402 to test branch and tag refs
 - cvsserver: support -r and sticky tags for most operations
 - cvsserver: Add version awareness to argsfromdir
 - cvsserver: generalize getmeta() to recognize commit refs
 - cvsserver: implement req_Sticky and related utilities
 - cvsserver: add misc commit lookup, file meta data, and file listing functions
 - cvsserver: define a tag name character escape mechanism
 - cvsserver: cleanup extra slashes in filename arguments
 - cvsserver: factor out git-log parsing logic

 Needs review by folks interested in cvsserver.


* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
 - config: exit on error accessing any config file
 - doc: advertise GIT_CONFIG_NOSYSTEM
 - config: treat user and xdg config permission problems as errors
 - config, gitignore: failure to access with ENOTDIR is ok

 An RFC to deal with a situation where .config/git is a file and we
 notice .config/git/config is not readable due to ENOTDIR, not
 ENOENT; I think a bit more refactored approach to consistently
 address permission errors across config, exclude and attrs may be
 desirable.

 Should we merge this as-is and build on top?  What are the chances
 of potential regressions?


* as/check-ignore (2012-11-08) 14 commits
 - t0007: fix tests on Windows
 - Documentation/check-ignore: we show the deciding match, not the first
 - Add git-check-ignore sub-command
 - dir.c: provide free_directory() for reclaiming dir_struct memory
 - pathspec.c: move reusable code from builtin/add.c
 - dir.c: refactor treat_gitlinks()
 - dir.c: keep track of where patterns came from
 - dir.c: refactor is_path_excluded()
 - dir.c: refactor is_excluded()
 - dir.c: refactor is_excluded_from_list()
 - dir.c: rename excluded() to is_excluded()
 - dir.c: rename excluded_from_list() to is_excluded_from_list()
 - dir.c: rename path_excluded() to is_path_excluded()
 - dir.c: rename cryptic 'which' variable to more consistent name

 Duy helped to reroll this, but it seems that there weren't any
 activity since then during my absense.


* fc/remote-testgit-feature-done (2012-10-29) 1 commit
 - remote-testgit: properly check for errors

 Is this still in "Needs review" state?  Are people involved in the
 remote interface happy with this change?


* jk/send-email-sender-prompt (2012-11-15) 8 commits
 - send-email: do not prompt for explicit repo ident
 - Git.pm: teach "ident" to query explicitness
 - var: provide explicit/implicit ident information
 - var: accept multiple variables on the command line
 - ident: keep separate "explicit" flags for author and committer
 - ident: make user_ident_explicitly_given static
 - t7502: factor out autoident prerequisite
 - test-lib: allow negation of prerequisites

 Avoid annoying sender prompt in git-send-email, but only when it is
 safe to do so.

 Perhaps keep only the first three patches, and replace the rest
 with the one from Felipe that takes a much simpler approach (the
 rationale of that patch needs to be cleaned up first, along the
 lines Jeff outlined, though).  Frozen until that happens.


* nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
 - Unify appending signoff in format-patch, commit and sequencer

 I am not sure if the logic to refrain from adding a sign-off based
 on the existing run of sign-offs is done correctly in this change.


* rc/maint-complete-git-p4 (2012-09-24) 1 commit
  (merged to 'next' on 2012-10-29 at af52cef)
 + Teach git-completion about git p4

 Comment from Pete will need to be addressed in a follow-up patch.


* as/test-tweaks (2012-09-20) 7 commits
 - tests: paint unexpectedly fixed known breakages in bold red
 - tests: test the test framework more thoroughly
 - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
 - tests: refactor mechanics of testing in a sub test-lib
 - tests: paint skipped tests in bold blue
 - tests: test number comes first in 'not ok $count - $message'
 - tests: paint known breakages in bold yellow

 Various minor tweaks to the test framework to paint its output
 lines in colors that match what they mean better.

 Has the "is this really blue?" issue Peff raised resolved???


* mk/maint-graph-infinity-loop (2012-09-25) 1 commit
 - graph.c: infinite loop in git whatchanged --graph -m

 The --graph code fell into infinite loop when asked to do what the
 code did not expect ;-)

 Anybody who worked on "--graph" wants to comment?
 Stalled mostly due to lack of responses.


* mb/remote-default-nn-origin (2012-07-11) 6 commits
 - Teach get_default_remote to respect remote.default.
 - Test that plain "git fetch" uses remote.default when on a detached HEAD.
 - Teach clone to set remote.default.
 - Teach "git remote" about remote.default.
 - Teach remote.c about the remote.default configuration setting.
 - Rename remote.c's default_remote_name static variables.

 When the user does not specify what remote to interact with, we
 often attempt to use 'origin'.  This can now be customized via a
 configuration variable.

 Expecting a re-roll.

 "The first remote becomes the default" bit is better done as a
 separate step.


* mh/ceiling (2012-10-29) 8 commits
 - string_list_longest_prefix(): remove function
 - setup_git_directory_gently_1(): resolve symlinks in ceiling paths
 - longest_ancestor_length(): require prefix list entries to be normalized
 - longest_ancestor_length(): take a string_list argument for prefixes
 - longest_ancestor_length(): use string_list_split()
 - Introduce new function real_path_if_valid()
 - real_path_internal(): add comment explaining use of cwd
 - Introduce new static function real_path_internal()

 Elements of GIT_CEILING_DIRECTORIES list may not match the real
 pathname we obtain from getcwd(), leading the GIT_DIR discovery
 logic to escape the ceilings the user thought to have specified.

 I think the fear that this would regress the intended use case of
 the environment variable turned out to be unfounded during the
 discussion.

 Should we merge this as-is to 'next', cook for a while to make sure
 nobody screams?

Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
@ 2012-11-21  2:46 Paul Fox
  2012-11-21  9:27 ` Krzysztof Mazur
  0 siblings, 1 reply; 18+ messages in thread
From: Paul Fox @ 2012-11-21  2:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King

In-reply-to: <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org> (sfid-20121120_190548_379327_D3EE7D14)
References: <7vpq39up0m.fsf@alter.siamese.dyndns.org> <7vy5hvq1ey.fsf@alter.siamese.dyndns.org> <7vobirq0q2.fsf_-_@alter.siamese.dyndns.org> (sfid-20121120_190548_379327_D3EE7D14)
Fcc: outbox
--------

junio c hamano wrote:
 > Here is a list of stalled topics I am having trouble deciding what
 > to do (the default is to dismiss them around feature freeze).
...
 > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 > 
 >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 >  session, not realizing git will receive the signal. Since most editors
 >  will take over the terminal and will block SIGINT, this is not likely
 >  to confuse anyone.
 > 
 >  Some people raised issues with emacsclient, which are addressed by this
 >  re-roll. It should probably also handle SIGQUIT, and there were a
 >  handful of other review comments.
 > 
 >  Anybody interested in moving this forward?

i started this, but then jeff took it and ran with it and made it
right.  i think the remaining changes are small -- if jeff would
prefer, i can probably finish it.  (but i won't guarantee not to
mess up the "From:" lines.  :-)

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 36.0 degrees)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21  0:05   ` Topics currently in the Stalled category Junio C Hamano
@ 2012-11-21  3:31     ` Felipe Contreras
  2012-11-28  2:59       ` Jeff King
  2012-11-21 14:55     ` Marc Branchaud
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Felipe Contreras @ 2012-11-21  3:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Here is a list of stalled topics I am having trouble deciding what
> to do (the default is to dismiss them around feature freeze).
>
> * fc/fast-export-fixes (2012-11-08) 14 commits
>
>  Renaming of remote-testgit feels to be a mistake.  It probably
>  should keep its source in remote-testgit.bash and generate it,

Why generate it? There's nothing to generate. python's source code
needs regeneration, bash's code doesn't.

> and
>  moreover, if it wants to rename remote-testgit.py to remote-testpy,
>  the new one should be called remote-testbash.

No, remote-testbash is not testing anything that is specific to bash,
it's testing the remote helper framework itself. It could be written
in Ruby, or Python, or C, or anything.

remote-testgit.py is *not* testing the remote helper framework, it's
testing the Python-specific remote helper framework.

IOW. remote-testgit tests this:

  transport-helper.c

remote-testpy tests this:

  git_remote_helpers/Makefile
  git_remote_helpers/__init__.py
  git_remote_helpers/git/__init__.py
  git_remote_helpers/git/exporter.py
  git_remote_helpers/git/git.py
  git_remote_helpers/git/importer.py
  git_remote_helpers/git/non_local.py
  git_remote_helpers/git/repo.py
  git_remote_helpers/setup.cfg
  git_remote_helpers/setup.py
  git_remote_helpers/util.py

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21  2:46 Topics currently in the Stalled category Paul Fox
@ 2012-11-21  9:27 ` Krzysztof Mazur
  2012-11-21 19:34   ` Jeff King
  0 siblings, 1 reply; 18+ messages in thread
From: Krzysztof Mazur @ 2012-11-21  9:27 UTC (permalink / raw)
  To: Paul Fox; +Cc: Junio C Hamano, git, Jeff King

On Tue, Nov 20, 2012 at 09:46:47PM -0500, Paul Fox wrote:
> junio c hamano wrote:
>  > Here is a list of stalled topics I am having trouble deciding what
>  > to do (the default is to dismiss them around feature freeze).
> ...
>  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
>  > 
>  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
>  >  session, not realizing git will receive the signal. Since most editors
>  >  will take over the terminal and will block SIGINT, this is not likely
>  >  to confuse anyone.
>  > 
>  >  Some people raised issues with emacsclient, which are addressed by this
>  >  re-roll. It should probably also handle SIGQUIT, and there were a
>  >  handful of other review comments.
>  > 
>  >  Anybody interested in moving this forward?
> 
> i started this, but then jeff took it and ran with it and made it
> right.  i think the remaining changes are small -- if jeff would
> prefer, i can probably finish it.  (but i won't guarantee not to
> mess up the "From:" lines.  :-)
> 

I'm also interested. I sometimes use ":r !command" in vim, so far I never
needed to use Ctrl-C, but maybe in future.

The SIGINT part seems to be finished, we need to decide what about SIGQUIT.

Krzysiek

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21  0:05   ` Topics currently in the Stalled category Junio C Hamano
  2012-11-21  3:31     ` Felipe Contreras
@ 2012-11-21 14:55     ` Marc Branchaud
  2012-11-22 11:24     ` Nguyen Thai Ngoc Duy
  2012-12-01  0:36     ` Adam Spiers
  3 siblings, 0 replies; 18+ messages in thread
From: Marc Branchaud @ 2012-11-21 14:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On 12-11-20 07:05 PM, Junio C Hamano wrote:
> Here is a list of stalled topics I am having trouble deciding what
> to do (the default is to dismiss them around feature freeze).
> 

[ snip ]

> 
> * mb/remote-default-nn-origin (2012-07-11) 6 commits
>  - Teach get_default_remote to respect remote.default.
>  - Test that plain "git fetch" uses remote.default when on a detached HEAD.
>  - Teach clone to set remote.default.
>  - Teach "git remote" about remote.default.
>  - Teach remote.c about the remote.default configuration setting.
>  - Rename remote.c's default_remote_name static variables.
> 
>  When the user does not specify what remote to interact with, we
>  often attempt to use 'origin'.  This can now be customized via a
>  configuration variable.
> 
>  Expecting a re-roll.
> 
>  "The first remote becomes the default" bit is better done as a
>  separate step.

This is still on my list of things to do soon.  Unfortunately "soon" seems to
be rather perpetual these days.

If you're tired of carrying the branch feel free to dismiss it and I'll
resurrect the topic when "soon" finally comes around.

		M.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21  9:27 ` Krzysztof Mazur
@ 2012-11-21 19:34   ` Jeff King
  2012-11-21 19:50     ` Paul Fox
  2012-11-21 19:53     ` Junio C Hamano
  0 siblings, 2 replies; 18+ messages in thread
From: Jeff King @ 2012-11-21 19:34 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: Paul Fox, Junio C Hamano, git

On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:

> >  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
> >  > 
> >  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
> >  >  session, not realizing git will receive the signal. Since most editors
> >  >  will take over the terminal and will block SIGINT, this is not likely
> >  >  to confuse anyone.
> >  > 
> >  >  Some people raised issues with emacsclient, which are addressed by this
> >  >  re-roll. It should probably also handle SIGQUIT, and there were a
> >  >  handful of other review comments.
> >  > 
> >  >  Anybody interested in moving this forward?
> > 
> > i started this, but then jeff took it and ran with it and made it
> > right.  i think the remaining changes are small -- if jeff would
> > prefer, i can probably finish it.  (but i won't guarantee not to
> > mess up the "From:" lines.  :-)
> > 
> 
> I'm also interested. I sometimes use ":r !command" in vim, so far I never
> needed to use Ctrl-C, but maybe in future.
> 
> The SIGINT part seems to be finished, we need to decide what about SIGQUIT.

My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think
there may have been one or two other minor comments in response to the
series). I am on vacation this week, but can revisit it next week. If
somebody wants to re-roll it in the meantime, that would be fine with
me.

-Peff

[1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
    nearly as widely used as SIGINT, but it sounds more like the
    principle of least surprise to treat them the same.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21 19:34   ` Jeff King
@ 2012-11-21 19:50     ` Paul Fox
  2012-11-21 19:53     ` Junio C Hamano
  1 sibling, 0 replies; 18+ messages in thread
From: Paul Fox @ 2012-11-21 19:50 UTC (permalink / raw)
  To: Jeff King; +Cc: Krzysztof Mazur, git

jeff wrote:
 > On Wed, Nov 21, 2012 at 10:27:50AM +0100, Krzysztof Mazur wrote:
 > 
 > > >  > * pf/editor-ignore-sigint (2012-11-11) 5 commits
 > > >  > 
 > > >  >  Avoid confusing cases where the user hits Ctrl-C while in the editor
 > > >  >  session, not realizing git will receive the signal. Since most editors
 > > >  >  will take over the terminal and will block SIGINT, this is not likely
 > > >  >  to confuse anyone.
 > > >  > 
 > > >  >  Some people raised issues with emacsclient, which are addressed by this
 > > >  >  re-roll. It should probably also handle SIGQUIT, and there were a
 > > >  >  handful of other review comments.
 > > >  > 
 > > >  >  Anybody interested in moving this forward?
 > > > 
 > > > i started this, but then jeff took it and ran with it and made it
 > > > right.  i think the remaining changes are small -- if jeff would
 > > > prefer, i can probably finish it.  (but i won't guarantee not to
 > > > mess up the "From:" lines.  :-)
 > > > 
 > > 
 > > I'm also interested. I sometimes use ":r !command" in vim, so far I never
 > > needed to use Ctrl-C, but maybe in future.
 > > 
 > > The SIGINT part seems to be finished, we need to decide what about SIGQUIT.
 > 
 > My plan was to just add in SIGQUIT[1] alongside SIGINT (and I think
 > there may have been one or two other minor comments in response to the
 > series). I am on vacation this week, but can revisit it next week. If
 > somebody wants to re-roll it in the meantime, that would be fine with
 > me.
 > 
 > -Peff
 > 
 > [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
 >     nearly as widely used as SIGINT, but it sounds more like the
 >     principle of least surprise to treat them the same.

i see no real point in treating them the same -- as you suggest, one
would only use SIGQUIT if SIGINT didn't work, and then you'd want them
to be treated differently.  so i'd be happy with the current code in
that regard.

i think krzysiek said that since editors usually catch SIGQUIT, git
should kill the editor when receiving SIGQUIT, essentially translating
the SIGQUIT to SIGTERM for the editor.  (please correct me if i
misunderstood.)  since well-behaved editors will die quickly anyway
(they get EIO on their next read from stdin), i'm not sure there's a
compelling reason for that extra step.

but i have no real objection to that behavior if others think it's
right -- there's certainly logic in saying that if git dies it should
ensure the editor does too.

(i'm away for the rest of the week also.)

paul
=---------------------
 paul fox, pgf@foxharp.boston.ma.us (arlington, ma, where it's 44.6 degrees)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21 19:34   ` Jeff King
  2012-11-21 19:50     ` Paul Fox
@ 2012-11-21 19:53     ` Junio C Hamano
  2012-11-21 20:13       ` Jeff King
  1 sibling, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2012-11-21 19:53 UTC (permalink / raw)
  To: Jeff King; +Cc: Krzysztof Mazur, Paul Fox, git

Jeff King <peff@peff.net> writes:

> [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
>     nearly as widely used as SIGINT, but it sounds more like the
>     principle of least surprise to treat them the same.

Sounds sensible.  I wonder what happens when the editor is suspended
;-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21 19:53     ` Junio C Hamano
@ 2012-11-21 20:13       ` Jeff King
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2012-11-21 20:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Krzysztof Mazur, Paul Fox, git

On Wed, Nov 21, 2012 at 11:53:04AM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > [1] Given the core-dumping behavior of SIGQUIT, I suspect it is not
> >     nearly as widely used as SIGINT, but it sounds more like the
> >     principle of least surprise to treat them the same.
> 
> Sounds sensible.  I wonder what happens when the editor is suspended
> ;-)

I think we would want to leave SIGTSTP alone; the editor should
typically respect it, and we would want to also pause until we get
SIGCONT (although even if we did continue, we would just be blocking on
wait() for the editor, anyway, so it is not a big deal).

Implicit in my "least surprise" comment above is that handling SIGQUIT
would match what system(3) does, so it makes sense to me to match that
(it also blocks SIGCHLD, but I do not think that really matters from a
user-visible perspective).

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21  0:05   ` Topics currently in the Stalled category Junio C Hamano
  2012-11-21  3:31     ` Felipe Contreras
  2012-11-21 14:55     ` Marc Branchaud
@ 2012-11-22 11:24     ` Nguyen Thai Ngoc Duy
  2012-12-01  0:36     ` Adam Spiers
  3 siblings, 0 replies; 18+ messages in thread
From: Nguyen Thai Ngoc Duy @ 2012-11-22 11:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wed, Nov 21, 2012 at 7:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/unify-appending-of-s-o-b (2012-11-15) 1 commit
>  - Unify appending signoff in format-patch, commit and sequencer
>
>  I am not sure if the logic to refrain from adding a sign-off based
>  on the existing run of sign-offs is done correctly in this change.

I'm not sure what's the right logic here. The original code in
format-patch has exactly one test just to verify that --signoff does
_something_. There's another signoff code in git-am.sh and we should
make sure at least the behavior is consistent. I guess I should start
over by writing new tests to record the current behavior, then fixes
and finally the unification. I think you can drop this for now.
-- 
Duy

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21  3:31     ` Felipe Contreras
@ 2012-11-28  2:59       ` Jeff King
  2012-11-28  3:10         ` Felipe Contreras
  2012-11-28  3:11         ` Jeff King
  0 siblings, 2 replies; 18+ messages in thread
From: Jeff King @ 2012-11-28  2:59 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git

On Wed, Nov 21, 2012 at 04:31:11AM +0100, Felipe Contreras wrote:

> On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Here is a list of stalled topics I am having trouble deciding what
> > to do (the default is to dismiss them around feature freeze).
> >
> > * fc/fast-export-fixes (2012-11-08) 14 commits
> >
> >  Renaming of remote-testgit feels to be a mistake.  It probably
> >  should keep its source in remote-testgit.bash and generate it,
> 
> Why generate it? There's nothing to generate. python's source code
> needs regeneration, bash's code doesn't.

We fix up the #!-lines on all of the existing shell scripts (as well as
python and perl). Wouldn't we want to do the same for people who have
bash in an alternate location?

As the series is now, people with bash in their PATH, but not in
/bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
git-remote-testgit hardcodes /bin/bash).

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-28  2:59       ` Jeff King
@ 2012-11-28  3:10         ` Felipe Contreras
  2012-11-28  3:12           ` Jeff King
  2012-11-28  3:11         ` Jeff King
  1 sibling, 1 reply; 18+ messages in thread
From: Felipe Contreras @ 2012-11-28  3:10 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Wed, Nov 28, 2012 at 3:59 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 21, 2012 at 04:31:11AM +0100, Felipe Contreras wrote:
>
>> On Wed, Nov 21, 2012 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Here is a list of stalled topics I am having trouble deciding what
>> > to do (the default is to dismiss them around feature freeze).
>> >
>> > * fc/fast-export-fixes (2012-11-08) 14 commits
>> >
>> >  Renaming of remote-testgit feels to be a mistake.  It probably
>> >  should keep its source in remote-testgit.bash and generate it,
>>
>> Why generate it? There's nothing to generate. python's source code
>> needs regeneration, bash's code doesn't.
>
> We fix up the #!-lines on all of the existing shell scripts (as well as
> python and perl). Wouldn't we want to do the same for people who have
> bash in an alternate location?

'#!/usr/bin/env bash' should take care of people who have bash in an
alternate location, no?

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-28  2:59       ` Jeff King
  2012-11-28  3:10         ` Felipe Contreras
@ 2012-11-28  3:11         ` Jeff King
  2012-11-28  3:15           ` Felipe Contreras
  1 sibling, 1 reply; 18+ messages in thread
From: Jeff King @ 2012-11-28  3:11 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git

On Tue, Nov 27, 2012 at 09:59:28PM -0500, Jeff King wrote:

> > >  Renaming of remote-testgit feels to be a mistake.  It probably
> > >  should keep its source in remote-testgit.bash and generate it,
> > 
> > Why generate it? There's nothing to generate. python's source code
> > needs regeneration, bash's code doesn't.
> 
> We fix up the #!-lines on all of the existing shell scripts (as well as
> python and perl). Wouldn't we want to do the same for people who have
> bash in an alternate location?
> 
> As the series is now, people with bash in their PATH, but not in
> /bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
> git-remote-testgit hardcodes /bin/bash).

We could improve the test in t5801, but it is nice to let people on such
systems test it, as well. And the infrastructure might be useful if we
ever acquire more bash scripts.

There's a fair bit of boilerplate, but I think this squashable patch
would do it:

diff --git a/.gitignore b/.gitignore
index 46595db..3b636bd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -124,6 +124,7 @@
 /git-remote-ftps
 /git-remote-fd
 /git-remote-ext
+/git-remote-testgit
 /git-remote-testsvn
 /git-remote-testpy
 /git-repack
diff --git a/Makefile b/Makefile
index 68f1ac2..19f6fd2 100644
--- a/Makefile
+++ b/Makefile
@@ -424,6 +424,7 @@ PROGRAM_OBJS =
 PROGRAMS =
 SCRIPT_PERL =
 SCRIPT_PYTHON =
+SCRIPT_BASH =
 SCRIPT_SH =
 SCRIPT_LIB =
 TEST_PROGRAMS_NEED_X =
@@ -473,9 +474,12 @@ SCRIPT_PERL += git-svn.perl
 SCRIPT_PYTHON += git-remote-testpy.py
 SCRIPT_PYTHON += git-p4.py
 
+SCRIPT_BASH += git-remote-testgit.bash
+
 SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \
 	  $(patsubst %.perl,%,$(SCRIPT_PERL)) \
 	  $(patsubst %.py,%,$(SCRIPT_PYTHON)) \
+	  $(patsubst %.bash,%,$(SCRIPT_BASH)) \
 	  git-instaweb
 
 ETAGS_TARGET = TAGS
@@ -571,9 +575,13 @@ endif
 ifndef PYTHON_PATH
 	PYTHON_PATH = /usr/bin/python
 endif
+ifndef BASH_PATH
+	BASH_PATH = /bin/bash
+endif
 
 export PERL_PATH
 export PYTHON_PATH
+export BASH_PATH
 
 LIB_FILE=libgit.a
 XDIFF_LIB=xdiff/lib.a
@@ -1931,6 +1939,10 @@ ifeq ($(PYTHON_PATH),)
 NO_PYTHON=NoThanks
 endif
 
+ifeq ($(BASH_PATH),)
+NO_BASH=NoThanks
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -2006,6 +2018,7 @@ gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
+BASH_PATH_SQ = $(subst ','\'',$(BASH_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 DIFF_SQ = $(subst ','\'',$(DIFF))
 
@@ -2267,6 +2280,15 @@ $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
 	mv $@+ $@
 endif # NO_PYTHON
 
+ifndef NO_BASH
+$(patsubst %.bash,%,$(SCRIPT_BASH)):
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1s|#!.*/bash|#!$(BASH_PATH_SQ)|' \
+	    $@.bash >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
+endif
+
 configure: configure.ac GIT-VERSION-FILE
 	$(QUIET_GEN)$(RM) $@ $<+ && \
 	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -2599,6 +2621,7 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
+	@echo NO_BASH=\''$(subst ','\'',$(subst ','\'',$(NO_BASH)))'\' >>$@
 	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
 ifdef GIT_TEST_OPTS
 	@echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' >>$@
diff --git a/git-remote-testgit b/git-remote-testgit.bash
similarity index 100%
rename from git-remote-testgit
rename to git-remote-testgit.bash
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 456303b..d6ebc5e 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -7,7 +7,7 @@ test_description='Test remote-helper import and export commands'
 
 . ./test-lib.sh
 
-if ! type "${BASH-bash}" >/dev/null 2>&1; then
+if ! test_have_prereq BASH; then
 	skip_all='skipping remote-testgit tests, bash not available'
 	test_done
 fi
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 489bc80..5300fa9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -675,6 +675,7 @@ esac
 ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
 test -z "$NO_PERL" && test_set_prereq PERL
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
+test -z "$NO_BASH" && test_set_prereq BASH
 test -n "$USE_LIBPCRE" && test_set_prereq LIBPCRE
 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT
 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-28  3:10         ` Felipe Contreras
@ 2012-11-28  3:12           ` Jeff King
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2012-11-28  3:12 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git

On Wed, Nov 28, 2012 at 04:10:07AM +0100, Felipe Contreras wrote:

> >> Why generate it? There's nothing to generate. python's source code
> >> needs regeneration, bash's code doesn't.
> >
> > We fix up the #!-lines on all of the existing shell scripts (as well as
> > python and perl). Wouldn't we want to do the same for people who have
> > bash in an alternate location?
> 
> '#!/usr/bin/env bash' should take care of people who have bash in an
> alternate location, no?

If that is where there env is. I do not recall offhand if that is a
problem for anyone. Still, I don't think it is that big a deal to treat
it like we do other scripts, and provide the usual boilerplate (which
also gets us NO_BASH if you have an old or broken bash and want to skip
the tests). I just sent a patch.

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-28  3:11         ` Jeff King
@ 2012-11-28  3:15           ` Felipe Contreras
  2012-11-28  3:22             ` Jeff King
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Contreras @ 2012-11-28  3:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Wed, Nov 28, 2012 at 4:11 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 27, 2012 at 09:59:28PM -0500, Jeff King wrote:
>
>> > >  Renaming of remote-testgit feels to be a mistake.  It probably
>> > >  should keep its source in remote-testgit.bash and generate it,
>> >
>> > Why generate it? There's nothing to generate. python's source code
>> > needs regeneration, bash's code doesn't.
>>
>> We fix up the #!-lines on all of the existing shell scripts (as well as
>> python and perl). Wouldn't we want to do the same for people who have
>> bash in an alternate location?
>>
>> As the series is now, people with bash in their PATH, but not in
>> /bin/bash, will fail t5801 (the prereq to skip the test uses "type", but
>> git-remote-testgit hardcodes /bin/bash).
>
> We could improve the test in t5801, but it is nice to let people on such
> systems test it, as well. And the infrastructure might be useful if we
> ever acquire more bash scripts.
>
> There's a fair bit of boilerplate, but I think this squashable patch
> would do it:

Yeah, but I wonder what's the point of installing this script, it's
mostly for testing and reference, and to add a whole category for that
seems like overkill.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-28  3:15           ` Felipe Contreras
@ 2012-11-28  3:22             ` Jeff King
  2012-11-28  3:33               ` Felipe Contreras
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2012-11-28  3:22 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git

On Wed, Nov 28, 2012 at 04:15:12AM +0100, Felipe Contreras wrote:

> > We could improve the test in t5801, but it is nice to let people on such
> > systems test it, as well. And the infrastructure might be useful if we
> > ever acquire more bash scripts.
> >
> > There's a fair bit of boilerplate, but I think this squashable patch
> > would do it:
> 
> Yeah, but I wonder what's the point of installing this script, it's
> mostly for testing and reference, and to add a whole category for that
> seems like overkill.

There's no point in installing it; I just didn't make the effort to
avoid doing so (note that testpy and testsvn are also installed, which
are in the same boat; it might make sense to split them all out like we
do for $TEST_PROGRAMS).

I agree it's an annoying amount of boilerplate, but it seems simpler
cognitively to me for it to behave as the other SCRIPT_* builds than to
do something simple but inconsistent.

I do not care enough to argue about it. We need to do something to fix
the impending test breakage on systems like Solaris. I have posted the
patch to handle BASH_PATH, so do what you want.

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-28  3:22             ` Jeff King
@ 2012-11-28  3:33               ` Felipe Contreras
  0 siblings, 0 replies; 18+ messages in thread
From: Felipe Contreras @ 2012-11-28  3:33 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git

On Wed, Nov 28, 2012 at 4:22 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 28, 2012 at 04:15:12AM +0100, Felipe Contreras wrote:
>
>> > We could improve the test in t5801, but it is nice to let people on such
>> > systems test it, as well. And the infrastructure might be useful if we
>> > ever acquire more bash scripts.
>> >
>> > There's a fair bit of boilerplate, but I think this squashable patch
>> > would do it:
>>
>> Yeah, but I wonder what's the point of installing this script, it's
>> mostly for testing and reference, and to add a whole category for that
>> seems like overkill.
>
> There's no point in installing it; I just didn't make the effort to
> avoid doing so (note that testpy and testsvn are also installed, which
> are in the same boat; it might make sense to split them all out like we
> do for $TEST_PROGRAMS).
>
> I agree it's an annoying amount of boilerplate, but it seems simpler
> cognitively to me for it to behave as the other SCRIPT_* builds than to
> do something simple but inconsistent.
>
> I do not care enough to argue about it. We need to do something to fix
> the impending test breakage on systems like Solaris. I have posted the
> patch to handle BASH_PATH, so do what you want.

I'm not objecting to the change, I'm simply wondering.

Personally I think switching to '/usr/bin/env bash' should be enough
for now. Doing a grep on my git installation throws this:

% grep '/usr/bin/env' -r /opt/git
/opt/git/lib/python2.7/site-packages/git_remote_helpers/git/git.py:#!/usr/bin/env
python
/opt/git/lib/python2.7/site-packages/git_remote_helpers/__init__.py:#!/usr/bin/env
python
/opt/git/lib/python2.7/site-packages/git_remote_helpers/util.py:#!/usr/bin/env
python
/opt/git/libexec/git-core/git-instaweb:#!/usr/bin/env ruby

So it's doubtful a lack of /usr/bin/env would cause any more breakages
than it already does for test-git.

And this just landed on 'pu', I don't think there's anything big
impending. If the /usr/bin/env solution turned out to be not enough
for some reason, we can deal with it then. That's my opinion.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Topics currently in the Stalled category
  2012-11-21  0:05   ` Topics currently in the Stalled category Junio C Hamano
                       ` (2 preceding siblings ...)
  2012-11-22 11:24     ` Nguyen Thai Ngoc Duy
@ 2012-12-01  0:36     ` Adam Spiers
  3 siblings, 0 replies; 18+ messages in thread
From: Adam Spiers @ 2012-12-01  0:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wed, Nov 21, 2012 at 12:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Here is a list of stalled topics I am having trouble deciding what
> to do (the default is to dismiss them around feature freeze).

[snipped]

> * as/check-ignore (2012-11-08) 14 commits
>  - t0007: fix tests on Windows
>  - Documentation/check-ignore: we show the deciding match, not the first
>  - Add git-check-ignore sub-command
>  - dir.c: provide free_directory() for reclaiming dir_struct memory
>  - pathspec.c: move reusable code from builtin/add.c
>  - dir.c: refactor treat_gitlinks()
>  - dir.c: keep track of where patterns came from
>  - dir.c: refactor is_path_excluded()
>  - dir.c: refactor is_excluded()
>  - dir.c: refactor is_excluded_from_list()
>  - dir.c: rename excluded() to is_excluded()
>  - dir.c: rename excluded_from_list() to is_excluded_from_list()
>  - dir.c: rename path_excluded() to is_path_excluded()
>  - dir.c: rename cryptic 'which' variable to more consistent name
>
>  Duy helped to reroll this, but it seems that there weren't any
>  activity since then during my absense.

I have been delayed several times, but I finally resumed work on
another re-roll.  I don't think there is any major reworking required;
just a number of small tweaks.

> * as/test-tweaks (2012-09-20) 7 commits
>  - tests: paint unexpectedly fixed known breakages in bold red
>  - tests: test the test framework more thoroughly
>  - [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
>  - tests: refactor mechanics of testing in a sub test-lib
>  - tests: paint skipped tests in bold blue
>  - tests: test number comes first in 'not ok $count - $message'
>  - tests: paint known breakages in bold yellow
>
>  Various minor tweaks to the test framework to paint its output
>  lines in colors that match what they mean better.
>
>  Has the "is this really blue?" issue Peff raised resolved???

I have a re-roll of this ready - just need to rebase to latest master,
do a final sanity check, and then send.

Sorry again for the delays.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-12-01  0:37 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-21  2:46 Topics currently in the Stalled category Paul Fox
2012-11-21  9:27 ` Krzysztof Mazur
2012-11-21 19:34   ` Jeff King
2012-11-21 19:50     ` Paul Fox
2012-11-21 19:53     ` Junio C Hamano
2012-11-21 20:13       ` Jeff King
  -- strict thread matches above, loose matches on Subject: below --
2012-11-19 23:55 What's cooking in git.git (Nov 2012, #06; Mon, 19) Junio C Hamano
2012-11-20 23:50 ` Junio C Hamano
2012-11-21  0:05   ` Topics currently in the Stalled category Junio C Hamano
2012-11-21  3:31     ` Felipe Contreras
2012-11-28  2:59       ` Jeff King
2012-11-28  3:10         ` Felipe Contreras
2012-11-28  3:12           ` Jeff King
2012-11-28  3:11         ` Jeff King
2012-11-28  3:15           ` Felipe Contreras
2012-11-28  3:22             ` Jeff King
2012-11-28  3:33               ` Felipe Contreras
2012-11-21 14:55     ` Marc Branchaud
2012-11-22 11:24     ` Nguyen Thai Ngoc Duy
2012-12-01  0:36     ` Adam Spiers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).