Summary: | ASTERISK-13797: [patch] relax badshell tilde test | ||||
Reporter: | Tzafrir Cohen (tzafrir) | Labels: | |||
Date Opened: | 2009-03-21 19:05:44 | Date Closed: | 2014-10-12 02:51:50 | ||
Priority: | Minor | Regression? | No | ||
Status: | Closed/Complete | Components: | Core/BuildSystem | ||
Versions: | Frequency of Occurrence | ||||
Related Issues: |
| ||||
Environment: | Attachments: | ( 0) tilde-destdir.diff | |||
Description: | The main Makefile has a target test called 'badshell' that tests if DESTDIR does not happen to have an an-expanded tilde (~). This might be the case if you run: make install DESTDIR=~/somewhere/ and the shell wouldn't expand it as expected due to either bugs or lack of features in it. However for packaging the Debian package we have, as part of the package name, '~dfsg' and sometimes also '~rcN'. This is because '~' is a special character for dpkg: 1.0~1 is a number smaller than 1.0 . Thus I suggest to relax the test a bit: check for '~/' rather than simply for '~'. ****** ADDITIONAL INFORMATION ****** This patch is for 1.6.1.0-rc3, though the change is trivial and can be manually applied to other versions. | ||||
Comments: | By: Tilghman Lesher (tilghman) 2009-03-22 11:27:47 I think I'd like a comment added to the Makefile, explaining this, so that we replicate it and don't inadvertently revert this progress in a future rewrite. By: Jason Parker (jparker) 2009-03-23 12:40:10 When I originally found this, my initial fix was ~/ instead of ~. Between testing and committing, it got changed to just ~. Looking back through IRC logs, I can't figure out why I removed the /. Would you be willing to test this on bash, dash, and csh? If those work, I'm perfectly fine with this. By: Tzafrir Cohen (tzafrir) 2009-03-23 12:57:00 Seems that the good behaviour is bash-specific. dash, posh, zsh and csh don't behave that way: tzafrir@sweetmorn:~$ bash tzafrir@sweetmorn:~$ echo ~ /home/tzafrir tzafrir@sweetmorn:~$ echo make DESTDIR=~ make DESTDIR=/home/tzafrir tzafrir@sweetmorn:~$ echo make DESTDIR=~/whatever make DESTDIR=/home/tzafrir/whatever tzafrir@sweetmorn:~$ dash $ echo ~ /home/tzafrir $ echo make DESTDIR=~ make DESTDIR=~ $ echo make DESTDIR=~/whatever make DESTDIR=~/whatever tzafrir@sweetmorn:~$ posh $ echo ~ /home/tzafrir $ echo make DESTDIR=~ make DESTDIR=~ $ echo make DESTDIR=~/whatever make DESTDIR=~/whatever tzafrir@sweetmorn:~$ zsh sweetmorn% echo ~ /home/tzafrir sweetmorn% echo make DESTDIR=~ make DESTDIR=~ sweetmorn% echo make DESTDIR=~/whatever make DESTDIR=~/whatever tzafrir@sweetmorn:~$ csh sweetmorn:~> echo ~ /home/tzafrir sweetmorn:~> echo make DESTDIR=~ make DESTDIR=~ sweetmorn:~> echo make DESTDIR=~/whatever make DESTDIR=~/whatever By: Jason Parker (jparker) 2009-03-23 13:05:49 Would it be possible to make the test check that ~ is at the start of the path? It seems that with your change and DESTDIR=~, things could still break. By: Tzafrir Cohen (tzafrir) 2009-03-23 13:17:15 I'm not sure it could be done using pure make text functions. It can surely be done with a $(shell) . By: Tilghman Lesher (tilghman) 2009-08-24 17:10:44 What about: ifneq ($(filter ~/%,$(DESTDIR)),) Just verified that it properly fails on csh, at least. By: Tzafrir Cohen (tzafrir) 2009-08-24 17:25:51 Isn't it exactly as my attached findstring patch? Anyway: has anybody actually used '~' (alone) as a DESTDIR (as opposed to PREFIX)? There's always the workaround of explicitly using $HOME . Also recall that this badshell test is intended to help users. If it doesn't, it can be safely removed. By: Tilghman Lesher (tilghman) 2009-08-24 17:59:41 No, as Qwell suggested, filter ensures that the ~ starts at the beginning, whereas findstring can find a ~ anywhere within the string. By: Leif Madsen (lmadsen) 2009-09-22 08:33:12 Is there anything we can do to move this issue forward, or can we simply not resolve the issue in this manner? By: Leif Madsen (lmadsen) 2009-10-26 09:42:54 I'm closing this for now as there doesn't appear to be a resolution here. Perhaps it would be best to discuss this on the asterisk-dev mailing list to get an idea of how this should be resolved. |