| Submitter | Augie Fackler |
|---|---|
| Date | May 27, 2017, 12:27 a.m. |
| Message ID | <ffca36020b1032817a45.1495844851@augie-macbookpro2.roam.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/20951/ |
| State | Accepted |
| Headers | show |
Comments
Would it make sense to put this (and the required pkgbuild change) on
'stable' to avoid the issue from the first paragraph of the description?
With a name like `hg`, if the user has a line like `autoload
${^fpath}/*(N-.:t)`
in their zshrc, it will create a shell function named `hg` that will hide
the
actual hg command and make hg unusable.
I don't know how long this was out there as 'hg' instead of '_hg', so it
might be only 4.2 that was affected, and if we have a patch release it
would be nice to have less time where packagers might take the potentially
very-broken version, instead of waiting for 4.3 to get it out there?
On Fri, May 26, 2017 at 5:27 PM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Kyle Lippincott <spectral@google.com>
> # Date 1495830247 25200
> # Fri May 26 13:24:07 2017 -0700
> # Node ID ffca36020b1032817a451e0fb752b80bbf55d84a
> # Parent 599d022d2d51320b2d593152e6d87650ceaf1c06
> zsh_completion: install as _hg not hg
>
> The contrib/zsh_completion file itself says to name it _hg.
>
> With a name like `hg`, if the user has a line like `autoload
> ${^fpath}/*(N-.:t)`
> in their zshrc, it will create a shell function named `hg` that will hide
> the
> actual hg command and make hg unusable.
>
> Separately from that though, the underscore prefix makes it actually work.
> The
> zsh man page states:
>
> The convention for autoloaded functions used in completion is that they
> start with an underscore
>
> This does not seem to just be a "convention", though. With the ill-advised
> line
> removed from my zshrc and the file named
> `/usr/local/share/zsh/site-functions/hg` (without the underscore), these
> completions did not seem to get loaded and the ones from the zsh
> installation
> were loaded instead. If I renamed them to be
> `/usr/local/share/zsh/site-functions/_hg`, however, they were loaded.
>
> I manually tested the above statement by starting a new zsh instance with
> the
> file in `/usr/local/share/zsh/site-functions` with the following names:
> - As `hg`, `which _hg_labels` did not show anything
> - As `_hg`, `which _hg_labels` showed the expected function.
>
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -169,7 +169,7 @@ osx:
> # install zsh completions - this location appears to be
> # searched by default as of macOS Sierra.
> install -d build/mercurial/usr/local/share/zsh/site-functions/
> - install -m 0644 contrib/zsh_completion build/mercurial/usr/local/
> share/zsh/site-functions/hg
> + install -m 0644 contrib/zsh_completion build/mercurial/usr/local/
> share/zsh/site-functions/_hg
> # install bash completions - there doesn't appear to be a
> # place that's searched by default for bash, so we'll follow
> # the lead of Apple's git install and just put it in a
> diff --git a/tests/test-mac-packages.t b/tests/test-mac-packages.t
> --- a/tests/test-mac-packages.t
> +++ b/tests/test-mac-packages.t
> @@ -29,8 +29,8 @@ Spot-check some randomly selected files:
> ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.py 100644
> 0/0
> ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.pyc 100644
> 0/0
> ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.pyo 100644
> 0/0
> - $ grep zsh/site-functions/hg boms.txt | cut -d ' ' -f 1,2,3
> - ./usr/local/share/zsh/site-functions/hg 100644 0/0
> + $ grep zsh/site-functions/_hg boms.txt | cut -d ' ' -f 1,2,3
> + ./usr/local/share/zsh/site-functions/_hg 100644 0/0
> $ grep hg-completion.bash boms.txt | cut -d ' ' -f 1,2,3
> ./usr/local/hg/contrib/hg-completion.bash 100644 0/0
> $ egrep 'man[15]' boms.txt | cut -d ' ' -f 1,2,3
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
On Tue, May 30, 2017 at 8:44 PM, Kyle Lippincott <spectral@pewpew.net> wrote: > Would it make sense to put this (and the required pkgbuild change) on > 'stable' to avoid the issue from the first paragraph of the description? > > With a name like `hg`, if the user has a line like `autoload > ${^fpath}/*(N-.:t)` > in their zshrc, it will create a shell function named `hg` that will hide > the > actual hg command and make hg unusable. > > I don't know how long this was out there as 'hg' instead of '_hg', so it > might be only 4.2 that was affected, and if we have a patch release it would > be nice to have less time where packagers might take the potentially > very-broken version, instead of waiting for 4.3 to get it out there? Sounds good to me. Anyone object? We'd need to graft this patch and the OS X installer fix for the .hg ignore in pkgbuild to stable. > > On Fri, May 26, 2017 at 5:27 PM, Augie Fackler <raf@durin42.com> wrote: >> >> # HG changeset patch >> # User Kyle Lippincott <spectral@google.com> >> # Date 1495830247 25200 >> # Fri May 26 13:24:07 2017 -0700 >> # Node ID ffca36020b1032817a451e0fb752b80bbf55d84a >> # Parent 599d022d2d51320b2d593152e6d87650ceaf1c06 >> zsh_completion: install as _hg not hg >> >> The contrib/zsh_completion file itself says to name it _hg. >> >> With a name like `hg`, if the user has a line like `autoload >> ${^fpath}/*(N-.:t)` >> in their zshrc, it will create a shell function named `hg` that will hide >> the >> actual hg command and make hg unusable. >> >> Separately from that though, the underscore prefix makes it actually work. >> The >> zsh man page states: >> >> The convention for autoloaded functions used in completion is that >> they >> start with an underscore >> >> This does not seem to just be a "convention", though. With the ill-advised >> line >> removed from my zshrc and the file named >> `/usr/local/share/zsh/site-functions/hg` (without the underscore), these >> completions did not seem to get loaded and the ones from the zsh >> installation >> were loaded instead. If I renamed them to be >> `/usr/local/share/zsh/site-functions/_hg`, however, they were loaded. >> >> I manually tested the above statement by starting a new zsh instance with >> the >> file in `/usr/local/share/zsh/site-functions` with the following names: >> - As `hg`, `which _hg_labels` did not show anything >> - As `_hg`, `which _hg_labels` showed the expected function. >> >> diff --git a/Makefile b/Makefile >> --- a/Makefile >> +++ b/Makefile >> @@ -169,7 +169,7 @@ osx: >> # install zsh completions - this location appears to be >> # searched by default as of macOS Sierra. >> install -d build/mercurial/usr/local/share/zsh/site-functions/ >> - install -m 0644 contrib/zsh_completion >> build/mercurial/usr/local/share/zsh/site-functions/hg >> + install -m 0644 contrib/zsh_completion >> build/mercurial/usr/local/share/zsh/site-functions/_hg >> # install bash completions - there doesn't appear to be a >> # place that's searched by default for bash, so we'll follow >> # the lead of Apple's git install and just put it in a >> diff --git a/tests/test-mac-packages.t b/tests/test-mac-packages.t >> --- a/tests/test-mac-packages.t >> +++ b/tests/test-mac-packages.t >> @@ -29,8 +29,8 @@ Spot-check some randomly selected files: >> ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.py 100644 >> 0/0 >> ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.pyc 100644 >> 0/0 >> ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.pyo 100644 >> 0/0 >> - $ grep zsh/site-functions/hg boms.txt | cut -d ' ' -f 1,2,3 >> - ./usr/local/share/zsh/site-functions/hg 100644 0/0 >> + $ grep zsh/site-functions/_hg boms.txt | cut -d ' ' -f 1,2,3 >> + ./usr/local/share/zsh/site-functions/_hg 100644 0/0 >> $ grep hg-completion.bash boms.txt | cut -d ' ' -f 1,2,3 >> ./usr/local/hg/contrib/hg-completion.bash 100644 0/0 >> $ egrep 'man[15]' boms.txt | cut -d ' ' -f 1,2,3 >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > >
On Wed, 31 May 2017 14:16:30 -0400, Augie Fackler wrote: > On Tue, May 30, 2017 at 8:44 PM, Kyle Lippincott <spectral@pewpew.net> wrote: > > Would it make sense to put this (and the required pkgbuild change) on > > 'stable' to avoid the issue from the first paragraph of the description? > > > > With a name like `hg`, if the user has a line like `autoload > > ${^fpath}/*(N-.:t)` > > in their zshrc, it will create a shell function named `hg` that will hide > > the > > actual hg command and make hg unusable. > > > > I don't know how long this was out there as 'hg' instead of '_hg', so it > > might be only 4.2 that was affected, and if we have a patch release it would > > be nice to have less time where packagers might take the potentially > > very-broken version, instead of waiting for 4.3 to get it out there? > > Sounds good to me. Anyone object? We'd need to graft this patch and > the OS X installer fix for the .hg ignore in pkgbuild to stable. +1
On Thu, Jun 1, 2017 at 10:19 AM, Yuya Nishihara <yuya@tcha.org> wrote: > On Wed, 31 May 2017 14:16:30 -0400, Augie Fackler wrote: >> On Tue, May 30, 2017 at 8:44 PM, Kyle Lippincott <spectral@pewpew.net> wrote: >> > Would it make sense to put this (and the required pkgbuild change) on >> > 'stable' to avoid the issue from the first paragraph of the description? >> > >> > With a name like `hg`, if the user has a line like `autoload >> > ${^fpath}/*(N-.:t)` >> > in their zshrc, it will create a shell function named `hg` that will hide >> > the >> > actual hg command and make hg unusable. >> > >> > I don't know how long this was out there as 'hg' instead of '_hg', so it >> > might be only 4.2 that was affected, and if we have a patch release it would >> > be nice to have less time where packagers might take the potentially >> > very-broken version, instead of waiting for 4.3 to get it out there? >> >> Sounds good to me. Anyone object? We'd need to graft this patch and >> the OS X installer fix for the .hg ignore in pkgbuild to stable. > > +1 This is now done. Thanks!
Patch
diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile @@ -169,7 +169,7 @@ osx: # install zsh completions - this location appears to be # searched by default as of macOS Sierra. install -d build/mercurial/usr/local/share/zsh/site-functions/ - install -m 0644 contrib/zsh_completion build/mercurial/usr/local/share/zsh/site-functions/hg + install -m 0644 contrib/zsh_completion build/mercurial/usr/local/share/zsh/site-functions/_hg # install bash completions - there doesn't appear to be a # place that's searched by default for bash, so we'll follow # the lead of Apple's git install and just put it in a diff --git a/tests/test-mac-packages.t b/tests/test-mac-packages.t --- a/tests/test-mac-packages.t +++ b/tests/test-mac-packages.t @@ -29,8 +29,8 @@ Spot-check some randomly selected files: ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.py 100644 0/0 ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.pyc 100644 0/0 ./Library/Python/2.7/site-packages/mercurial/pure/bdiff.pyo 100644 0/0 - $ grep zsh/site-functions/hg boms.txt | cut -d ' ' -f 1,2,3 - ./usr/local/share/zsh/site-functions/hg 100644 0/0 + $ grep zsh/site-functions/_hg boms.txt | cut -d ' ' -f 1,2,3 + ./usr/local/share/zsh/site-functions/_hg 100644 0/0 $ grep hg-completion.bash boms.txt | cut -d ' ' -f 1,2,3 ./usr/local/hg/contrib/hg-completion.bash 100644 0/0 $ egrep 'man[15]' boms.txt | cut -d ' ' -f 1,2,3
# HG changeset patch # User Kyle Lippincott <spectral@google.com> # Date 1495830247 25200 # Fri May 26 13:24:07 2017 -0700 # Node ID ffca36020b1032817a451e0fb752b80bbf55d84a # Parent 599d022d2d51320b2d593152e6d87650ceaf1c06 zsh_completion: install as _hg not hg The contrib/zsh_completion file itself says to name it _hg. With a name like `hg`, if the user has a line like `autoload ${^fpath}/*(N-.:t)` in their zshrc, it will create a shell function named `hg` that will hide the actual hg command and make hg unusable. Separately from that though, the underscore prefix makes it actually work. The zsh man page states: The convention for autoloaded functions used in completion is that they start with an underscore This does not seem to just be a "convention", though. With the ill-advised line removed from my zshrc and the file named `/usr/local/share/zsh/site-functions/hg` (without the underscore), these completions did not seem to get loaded and the ones from the zsh installation were loaded instead. If I renamed them to be `/usr/local/share/zsh/site-functions/_hg`, however, they were loaded. I manually tested the above statement by starting a new zsh instance with the file in `/usr/local/share/zsh/site-functions` with the following names: - As `hg`, `which _hg_labels` did not show anything - As `_hg`, `which _hg_labels` showed the expected function.