From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Egger Subject: Re: [PATCH] tools/pygrub: do not override pygrub with a symbolic link Date: Mon, 22 Apr 2013 16:48:01 +0200 Message-ID: <51754DA1.4040600@amazon.de> References: <517523FA.7050303@amazon.de> <1366631923.22143.47.camel@zakaz.uk.xensource.com> <51752C82.7010308@amazon.de> <1366634055.22143.64.camel@zakaz.uk.xensource.com> <51753623.9010401@amazon.de> <1366641108.22143.65.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------010708080206070800070902" Return-path: In-Reply-To: <1366641108.22143.65.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel List-Id: xen-devel@lists.xenproject.org --------------010708080206070800070902 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit On 22.04.13 16:31, Ian Campbell wrote: > On Mon, 2013-04-22 at 14:07 +0100, Christoph Egger wrote: >> On 22.04.13 14:34, Ian Campbell wrote: >>> On Mon, 2013-04-22 at 13:26 +0100, Christoph Egger wrote: >>>> On 22.04.13 13:58, Ian Campbell wrote: >>>>> On Mon, 2013-04-22 at 12:50 +0100, Christoph Egger wrote: >>>>>> tools/pygrub: Do not override pygrub with a symbolic link if $(BINDIR) >>>>>> and $(PRIVATE_BINDIR) are the same. >>>>> >>>>> More properly this is "fix the if condition we use to decide when to >>>>> create the symlink", since it is already the intention to not override >>>>> if they are the same but the condition is written wrong since it >>>>> includes DESTDIR on one side but not the other, which defeats the check. >>>>> >>>>> FWIW I would niuke the DESTDIR on the left rather than adding it on the >>>>> right, it might even fit on one line then? >>>> >>>> Does not work. See comment below. >>> >>> OK, that makes sense I guess. >>> >>>>>> Signed-off-by: Christoph Egger >>>>>> diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile >>>>>> index 039f7f7..c3b34d7 100644 >>>>>> --- a/tools/pygrub/Makefile >>>>>> +++ b/tools/pygrub/Makefile >>>>>> @@ -15,8 +15,8 @@ install: all >>>>>> --install-scripts=$(PRIVATE_BINDIR) --force >>>>>> $(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot >>>>>> set -e; if [ "`readlink -f $(DESTDIR)/$(BINDIR)`" != \ >>>>>> - "`readlink -f $(PRIVATE_BINDIR)`" ]; then \ >>>>>> - ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \ >>>>>> + "`readlink -f $(DESTDIR)/$(PRIVATE_BINDIR)`" ]; then \ >>>>>> + ln -sf $(DESTDIR)/$(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \ >>>>> >>>>> I don't think this change to the first path passed to ln is correct >>>>> since this will become the content of the symlink, which doesn't want to >>>>> include DESTDIR. >>>> >>>> What actually happens when $(BINDIR) and $(PRIVATE_BINDIR) are equal >>>> is that the pygrub python script does not exist. pygrub is a symbolic >>>> link to a file that doesn't exist instead. >>>> >>>> With this patch you will actually install the pygrub python script >>>> rather a symbolic link. >>> >>> But you have broken the case where BINDIR and PRIVATE_BINDIR differ >>> because /usr/bin/pygrub will be a symlink >>> to /tmp/destdir.XYZ/usr/lib/xen/bin/pygrub and not >>> to /usr/lib/xen/bin/pygrub. >> >> I see. >> >>> >>> The code inside the if shouldn't even be run in the case you are trying >>> to fix. >> >> Right. Comparing $(DESTDIR)/$(BINDIR) with $(PRIVATE_BINDIR) is >> always different. Also when comparing $(BINDIR) with >> $(DESTDIR)/$(PRIVATE_BINDIR) is always different >> and the code inside the if runs. >> >> This works for me and should be fine for you: > > I think so too. Great. Attached this new version. tools/pygrub: Do not override pygrub with a symbolic link if $(BINDIR) and $(PRIVATE_BINDIR) are the same. Signed-off-by: Christoph Egger diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile index 039f7f7..0191638 100644 --- a/tools/pygrub/Makefile +++ b/tools/pygrub/Makefile @@ -14,7 +14,8 @@ install: all $(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \ --install-scripts=$(PRIVATE_BINDIR) --force $(INSTALL_DIR) $(DESTDIR)/var/run/xend/boot - set -e; if [ "`readlink -f $(DESTDIR)/$(BINDIR)`" != \ + set -e; if [ $(BINDIR) != $(PRIVATE_BINDIR) -a \ + "`readlink -f $(DESTDIR)/$(BINDIR)`" != \ "`readlink -f $(PRIVATE_BINDIR)`" ]; then \ ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \ fi Christoph --------------010708080206070800070902 Content-Type: text/plain; charset="UTF-8"; name="patch_pygrub.diff" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="patch_pygrub.diff" Y29tbWl0IDgwMDM0NzMxZmVlMzJiNGM0YzFjNWMzMjZhMjY1ZTZmNzU0NjlhOGYKQXV0aG9y OiBDaHJpc3RvcGggRWdnZXIgPGNoZWdnZXJAYW1hem9uLmRlPgpEYXRlOiAgIEZyaSBGZWIg OCAxMjo0NDoxMiAyMDEzICswMDAwCgogICAgdG9vbHMvcHlncnViOiBEbyBub3Qgb3ZlcnJp ZGUgcHlncnViIHdpdGggYSBzeW1ib2xpYyBsaW5rIGlmICQoQklORElSKQogICAgYW5kICQo UFJJVkFURV9CSU5ESVIpIGFyZSB0aGUgc2FtZS4KICAgIAogICAgU2lnbmVkLW9mZi1ieTog Q2hyaXN0b3BoIEVnZ2VyIDxjaGVnZ2VyQGFtYXpvbi5kZT4KCmRpZmYgLS1naXQgYS90b29s cy9weWdydWIvTWFrZWZpbGUgYi90b29scy9weWdydWIvTWFrZWZpbGUKaW5kZXggMDM5Zjdm Ny4uMDE5MTYzOCAxMDA2NDQKLS0tIGEvdG9vbHMvcHlncnViL01ha2VmaWxlCisrKyBiL3Rv b2xzL3B5Z3J1Yi9NYWtlZmlsZQpAQCAtMTQsNyArMTQsOCBAQCBpbnN0YWxsOiBhbGwKIAkJ JChQWVRIT05fUFJFRklYX0FSRykgLS1yb290PSIkKERFU1RESVIpIiBcCiAJCS0taW5zdGFs bC1zY3JpcHRzPSQoUFJJVkFURV9CSU5ESVIpIC0tZm9yY2UKIAkkKElOU1RBTExfRElSKSAk KERFU1RESVIpL3Zhci9ydW4veGVuZC9ib290Ci0Jc2V0IC1lOyBpZiBbICJgcmVhZGxpbmsg LWYgJChERVNURElSKS8kKEJJTkRJUilgIiAhPSBcCisJc2V0IC1lOyBpZiBbICQoQklORElS KSAhPSAkKFBSSVZBVEVfQklORElSKSAtYSBcCisJICAgICAgICAgICAgICJgcmVhZGxpbmsg LWYgJChERVNURElSKS8kKEJJTkRJUilgIiAhPSBcCiAJICAgICAgICAgICAgICJgcmVhZGxp bmsgLWYgJChQUklWQVRFX0JJTkRJUilgIiBdOyB0aGVuIFwKIAkgICAgbG4gLXNmICQoUFJJ VkFURV9CSU5ESVIpL3B5Z3J1YiAkKERFU1RESVIpLyQoQklORElSKTsgXAogCWZpCg== --------------010708080206070800070902 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------010708080206070800070902--