[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] tools/pygrub: do not override pygrub with a symbolic link
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 <chegger@xxxxxxxxx> >>>> 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: if [ $(BINDIR) != $(PRIVATE_BINDIR) -a \ "`readlink -f $(DESTDIR)/$(BINDIR)`" != \ "`readlink -f $(PRIVATE_BINDIR)`" ]; then \ ln -sf $(PRIVATE_BINDIR)/pygrub $(DESTDIR)/$(BINDIR); \ Christoph _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |