Hi,
I needed to patch some make files of x2goserver in order to be able to build rpm packages of it. It would be nice if you could evaluate them and see, if they can be merged upstream. Thanks,
Oliver
On Tue, Feb 7, 2012 at 2:09 PM, Oliver Burger <obgr_seneca@mageia.org> wrote:
Hi,
I needed to patch some make files of x2goserver in order to be able to build rpm packages of it. It would be nice if you could evaluate them and see, if they can be merged upstream. Thanks,
I wonder why do you require these changes? Don't you use something like fakeroot for package building? that should the INSTALL_OWNER changes unnecessary.
The SYSCONFDIR/ETCDIR might make sense if you want to install somewhere else than '$(DESTDIR)/etc/x2go'. However, I'm pretty sure that installing those files somewhere else will break x2go in more or less subtle ways as I think those paths are hardcoded at more other places. Therefore, I wonder again what this patch fixes for you.
-- regards, Reinhard
On Tue, Feb 7, 2012 at 2:09 PM, Oliver Burger<obgr_seneca@mageia.org> wrote:
I wonder why do you require these changes? Don't you use something like fakeroot for package building? that should the INSTALL_OWNER changes unnecessary. Well, I know, that fakeroot is used in deb packaging but not in rpm
Am 07.02.2012 14:52, schrieb Reinhard Tartler: packaging, and since you don't build as root, it's quite impossible to use those -g and -o options. In addition, they are not needed at all, the package building process does fix the permission of these files on its own, at least rpmbuild does.
The SYSCONFDIR/ETCDIR might make sense if you want to install somewhere else than '$(DESTDIR)/etc/x2go'. However, I'm pretty sure that installing those files somewhere else will break x2go in more or less subtle ways as I think those paths are hardcoded at more other places. Therefore, I wonder again what this patch fixes for you.
You are partly right there. While rediffing the patch from 3.0.99.9 I did overlook, the $(DESTDIR) was already added. The patch read
mkdir -p /etc/X11/Xresources
mkdir -p /etc/X11/Xsession.d
touch /etc/X11/Xsession.options
mkdir -p $(DESTDIR)$(SYSCONFDIR)/X11/Xresources
mkdir -p $(DESTDIR)$(SYSCONFDIR)/X11/Xsession.d
in the previous version.
So you are right, this one isn't really needed anymore.
But I have to wonder, why a variable $(ETCDIR) is defined instead of
hardcoding it, when it will break x2goserver, if the user should define
it during build?
Your argument there seems to fire backwards.Oliver
On Tue, Feb 7, 2012 at 3:32 PM, Oliver Burger <oliver.bgr@googlemail.com> wrote:
Am 07.02.2012 14:52, schrieb Reinhard Tartler:
On Tue, Feb 7, 2012 at 2:09 PM, Oliver Burger<obgr_seneca@mageia.org> wrote:
I wonder why do you require these changes? Don't you use something like fakeroot for package building? that should the INSTALL_OWNER changes unnecessary.
Well, I know, that fakeroot is used in deb packaging but not in rpm packaging, and since you don't build as root, it's quite impossible to use those -g and -o options.
Just curious, is fakeroot not available *at all* on fedora/mageia systems.
In addition, they are not needed at all, the package building process does fix the permission of these files on its own, at least rpmbuild does.
That maybe right, but if we go this route, your patch is also unnecessary, because you can also easily directly override the INSTALL_DIR and INSTALL_FILE macros during the makefile invocation/
The SYSCONFDIR/ETCDIR might make sense if you want to install somewhere else than '$(DESTDIR)/etc/x2go'. However, I'm pretty sure that installing those files somewhere else will break x2go in more or less subtle ways as I think those paths are hardcoded at more other places. Therefore, I wonder again what this patch fixes for you.
You are partly right there. While rediffing the patch from 3.0.99.9 I did overlook, the $(DESTDIR) was already added. The patch read
- in the previous version. So you are right, this one isn't really needed anymore. But I have to wonder, why a variable $(ETCDIR) is defined instead of hardcoding it, when it will break x2goserver, if the user should define it during build? Your argument there seems to fire backwards.
I never claimed the Makefile to be perfect.
If we wanted the to make the location /etc/x2go/... really configurable (which I think would be great) then this is mechanism is a great start. Nevertheless, a lot of additional places needs to be fixed, and I see a lot of other way more important things that need fixing first. Therefore, I'd personally recommend to focus on things that really need fixing first.
Your proposed patches in this thread seem rather cosmetic to me, because the functionality that you require can be achieved without any patching as well. I'm not saying they are bad or wrong, I'd rather prefer the makefile to be fixed properly instead of making it more complicated.
-- regards, Reinhard
2012/2/11 Reinhard Tartler <siretart@gmail.com>:
On Tue, Feb 7, 2012 at 3:32 PM, Oliver Burger <oliver.bgr@googlemail.com> wrote:
Well, I know, that fakeroot is used in deb packaging but not in rpm packaging, and since you don't build as root, it's quite impossible to use those -g and -o options.
Just curious, is fakeroot not available *at all* on fedora/mageia systems. Well, I just looked. Actually fakeroot is available, at least on Mageia. I have no Fedora system here right now. But I'm not aware of it normally being used. And I have to ask others about our - and the Fedora - build servers being able to use it or not.
About the other things, my patch proposes. of course it is possible to overwrite INSTALL_DIR, INSTALL_FILE and INSTALL_PROGRAMM, actually that didn't come to my mind, but it does make the make line rather long ;)
Oliver
On Sat, Feb 11, 2012 at 12:03 AM, Reinhard Tartler <siretart@gmail.com> wrote:
Your proposed patches in this thread seem rather cosmetic to me, because the functionality that you require can be achieved without any patching as well. I'm not saying they are bad or wrong, I'd rather prefer the makefile to be fixed properly instead of making it more complicated.
Is anyone working on fixing the makefiles in nx-libs (redistributed)?
The fixes I made to the nx-libs build system were certainly more than cosmetic, and they work (I've been using them for a month now), but nx-libs is still being distributed with a horribly broken imake build system. I certainly don't claim the fixes I made are perfect, but they do work, and they do install files in their proper places, and that is better than what is currently being distributed.
Hence, my question. Is someone working on fixing the nx-libs build files?
On Tue, Feb 7, 2012 at 2:09 PM, Oliver Burger<obgr_seneca@mageia.org> wrote:
I wonder why do you require these changes? Don't you use something like fakeroot for package building? that should the INSTALL_OWNER changes unnecessary. Well, I know, that fakeroot is used in deb packaging but not in rpm
Am 07.02.2012 14:52, schrieb Reinhard Tartler: packaging, and since you don't build as root, it's quite impossible to use those -g and -o options. In addition, they are not needed at all, the package building process does fix the permission of these files on its own, at least rpmbuild does.
The SYSCONFDIR/ETCDIR might make sense if you want to install somewhere else than '$(DESTDIR)/etc/x2go'. However, I'm pretty sure that installing those files somewhere else will break x2go in more or less subtle ways as I think those paths are hardcoded at more other places. Therefore, I wonder again what this patch fixes for you.
You are partly right there. While rediffing the patch from 3.0.99.9 I did overlook, the $(DESTDIR) was already added. The patch read
mkdir -p /etc/X11/Xresources
mkdir -p /etc/X11/Xsession.d
touch /etc/X11/Xsession.options
mkdir -p $(DESTDIR)$(SYSCONFDIR)/X11/Xresources
mkdir -p $(DESTDIR)$(SYSCONFDIR)/X11/Xsession.d
in the previous version.
So you are right, this one isn't really needed anymore.
But I have to wonder, why a variable $(ETCDIR) is defined instead of
hardcoding it, when it will break x2goserver, if the user should define
it during build?
Your argument there seems to fire backwards.Oliver