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