Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Makefile] Separate DESTDIR from the other VARIABLES #40

Open
tokiclover opened this issue Aug 15, 2015 · 23 comments
Open

[Makefile] Separate DESTDIR from the other VARIABLES #40

tokiclover opened this issue Aug 15, 2015 · 23 comments

Comments

@tokiclover
Copy link

Please separate DESTDIR variable from the other VARIABLES because it is the way do this kind of things; otherwise, it become quickqly cumbersome to modify any with prepending DESTDIR before system directory. See sys-libs/libpstat/files/Makefile-DESTDIR.path and or sys-fs/fskit/files/Makefile-DESTDIR.path or simply supervision as a sipme examle (which can be found in ubiquitous packages in the intertubes.)

Second, /usr should not be DESTDIR but only PREFIX! This one is very important because you should not confuse them. -- DESTDIR is usually not defined in 100% of makefile out there; and PREFIX has usually /usr/local value instead of simply /usr for compliancy, historic and manual installation reasons.

Thanks.

NOTE: Yes I am trying to make Gentoo packages!

@tokiclover
Copy link
Author

Sorry to have forgotten the links; fixed now.

@tokiclover
Copy link
Author

Of course this issue is valid for fskit and libpstat. Also forgotten in the first comment.

@jcnelson
Copy link
Owner

Confirmed; will take a look as soon as I can.

@tokiclover
Copy link
Author

To finish succintely, I can look at autotools/automake to make a configure.ac and Makefile.{am,in} and resolve many things with one shot because I do not have the time to grasp the internal of vdev to get what is necessary to patch the Makefile... Though this will require autotools/automake dependencies for the git tree; and generating Makefile(s) and configure script can be done for futur versioned tarballs. This will simplify or externalize too much handling of sub-dirs and sub-projects dependencies at hand which would be taken care with the power of autotools/automake.

What do you think about it? I could open an new issue later in case of a positive answer.

@jcnelson
Copy link
Owner

@tokiclover Feel free to do so downstream, but please be informed that I do not have the time or energy to maintain support for GNU autotools, nor do I believe they are necessary for vdev. I will not be accepting a pull request for adding support for GNU autotools to the build system in the foreseeable future.

Recall that the primary goal of GNU autotools is to provide a consistent build environment on wide variety of operating systems with minimal source modifications. It is my current judgement that this is unnecessary for vdev, since vdev is only going to support running on Linux (no less than 2.6.24) and later on OpenBSD (once the Linux port is stable). The added costs of maintaining support for a complex build system exceed the benefits it would yield.

I do not have the time to grasp the internal of vdev to get what is necessary to patch the Makefile

If you intend to add an alternative build system, you might want to take the time to understand vdev's current build system and code layout first :). I'm happy to answer any questions you might have on that matter.

@tokiclover
Copy link
Author

Come on @jcnelson... no need to write all that stuff instead of simply saying no to simple suggestion which would avoid wasting unnecessary time on both side. You should accept or refuse suggestions quite simply in a straight forward manner along with dealing with issues.

Well, my bad, lesson learned (from my side regarding the treatment of this suggestion and other issues.) I won't bother openning issues which aren't critical nor suggest anything beyond the immediat scope of vdev.

I wish you the best and hope other people would contribute to this project in order to get a worthy udev alternative. Good luck!

@jcnelson
Copy link
Owner

I wish you the best and hope other people would contribute to this project in order to get a worthy udev alternative. Good luck!

Thanks!

@jcnelson jcnelson reopened this Aug 17, 2015
@jcnelson
Copy link
Owner

(accidentally hit "close and comment")

@jcnelson
Copy link
Owner

Okay, I think this is fixed as of ffc1f06.

@tokiclover
Copy link
Author

Not quite... to be polite. You're forcing to define DESTDIR and then, one has to include the value of DESTDIR every time when appending another variable but PREFIX. PREFIX value should not include the value of DESTDIR, nor LIBDIR, ETCDIR (which is usually and commonly named CONFDIR instead) et al. Each variable has a specific meaning that are not intertwined together to make sense of defining theme in the first place.

Think about this. If you keep tangling those variables together; defining and generating vdev.pc (or/and libudev-compat.pc) will not be possible if one is needed in the future which is, by the way, likely to happen because of the presence of... libudev-compat. How are you going to separate the value of DESTDIR from LIBDIR et al once tied together? Are you going to include the value DESTDIR in the .pc file? Nonsense.

You might look at the Makefile from LZ4 which makes use of only a makefile. And the .pc file section is one of interest as well.

@jcnelson
Copy link
Owner

See c97e20c and 8317212.

@tokiclover
Copy link
Author

It appears... you're making funny commits by making cosmetic commits instead of... addressing the issue at hand. My bad... I did not know you will be reacting and doing this kind of cinema instead of accepting simple pointers, criticisms and issues kindly opened by users in order to improve this project. So what are expecting with this kind of bad joke?!

Ciao! I might never came here again depending on your response/actions in response to this. I don't want to waste my time looking at those kind commit just to kill some times. -- I do not need this kind of entertainment, no thanks.

NOTE: I don't know if I shall laugh or not... Yellow laugh if you ask me. LULZ.

@jcnelson
Copy link
Owner

@tokiclover I'm going by the documentation here [1]. Your issue was specifically with DESTDIR being separate from other variables, and I believe that my last two commits have made buildconf.mk consistent with sections 7.2.4 and 7.2.5. Is this not the case?

[1] https://www.gnu.org/prep/standards/html_node/Makefile-Conventions.html

@jvvv
Copy link

jvvv commented Aug 29, 2015

I'm not sure if this is what toki is referring to, but I did find that in buildconf.mk, USRSBINDIR and USRSHAREDIR are not defined. I looked through all of the makefiles looking for places where the install target might (accidently) install outside of DESTDIR if it's defined. The aforementioned undefined variables are all I spotted. While your makefile syntax usage is different than what I've seen in many autotooled or GNU sources (this isn't a bad thing, AFAIAC), they looked effective to my eyes. Once the USRSBINDIR and USRSHAREDIR are defined, then I think install targets should place everything into $DESTDIR, as expected.

@tokiclover
Copy link
Author

On Fri, Aug 28, 2015 at 02:52:20PM -0700, Jude Nelson wrote:

@tokiclover I'm going by the documentation here [1]. Your issue was specifically with DESTDIR being separate from other variables, and I believe that my last two commits have made buildconf.mk consistent with sections 7.2.4 and 7.2.5. Is this not the case?

[1] https://www.gnu.org/prep/standards/html_node/Makefile-Conventions.html

Yes, precisely! There is practicaly nothing changed, again, but the
removal of DESTDIR value from PREFIX.

And if you look at [1] ref. in the 7.2.4 DESTDIR sub-section, one can
see this kind of examles:

$(INSTALL_PROGRAM) foo $(DESTDIR)$(bindir)/foo
$(INSTALL_DATA) libfoo.a $(DESTDIR)$(libdir)/libfoo.a

If you look at it, one can pretty much see that prefix/PREFIX and and
libdir/LIBDIR are separated from those two directory variables, and
thus, are used in the installation rules contrary to your incorrect
usage (in buildconf.mk):

PREFIX         ?= /usr/local
INCLUDE_PREFIX ?= $(PREFIX)
BINDIR         ?= $(DESTDIR)$(PREFIX)/bin
SBINDIR         ?= $(DESTDIR)$(PREFIX)/sbin
LIBDIR         ?= $(DESTDIR)$(PREFIX)/lib
INCLUDEDIR     ?= $(DESTDIR)$(INCLUDE_PREFIX)/include
ETCDIR          ?= $(DESTDIR)$(PREFIX)/etc

incorrect usage of DESTDIR in the definition of other directory variables.

What do you want me to say when your link confirm my previous remarks
and Makefile still define the directory variables with the value of
DESTDIR after that many commits?

I've lost my Latin already after putting down so many words down just to
get ignored again and again.

@fbt
Copy link
Contributor

fbt commented Aug 29, 2015

You're being antagonistic for no good reason. There is a misunderstanding here, and if you want it to be fixed, explain it without being an ass.

On the actual topic: yes, using DESTDIR in anything but install targets is wrong and makes life for packagers harder by being basically useless.

I personally just was too lazy to go through the makefiles to fix it, as they are pretty complex.
I'm sure Jude would be perfectly fine with accepting a pull request that fixes his makefiles. Am I wrong?

@fbt
Copy link
Contributor

fbt commented Aug 29, 2015

I was going to leave this bit till vdev is almost ready to have a proper release, as it's not that important on the alpha stage, but let me explain DESTDIR from the point of view of a packager:

DESTDIR is the directory that would be root (/) on target systems. Which means that it must only be prepended to install targets and NEVER touch anything else inside the root. So, for example, the config makefile ends up getting DESTDIR into vdevd.conf, which is obviously wrong, and the current makefiles don't offer me an obvious way to override this without also breaking the packaging process:

acls=/home/fbt/git/pkg/vdev-git/pkg/vdev-git/usr/etc/vdev/acls
actions=/home/fbt/git/pkg/vdev-git/pkg/vdev-git/usr/etc/vdev/actions
ifnames=/home/fbt/git/pkg/vdev-git/pkg/vdev-git/usr/etc/vdev/ifnames.conf

Think of it this way: you want DESTDIR to not exist on the target machine the package is being built for. It's just a dir we put files into to make a snapshot from it. It must not interfere with anything else.

@fbt
Copy link
Contributor

fbt commented Aug 29, 2015

I've created a pull request with a partial fix to buildconf.mk.

@jcnelson
Copy link
Owner

@fbt: Thank you for your helpful explanation regarding DESTDIR, and thank you for your pull request. This makes a lot more sense to me, and I can see now why the buildconf.mk was incorrectly constructed. I have pushed some more patches to the build system that hopefully fix the remaining problems with libudev-compat and the config files. In particular, DESTDIR is no longer used to construct the fields in the config file.

@tokiclover: With @fbt's pull request, DESTDIR is now used to construct the INSTALL_* variables in the buildconf.mk file, which are then used to construct install targets in each Makefile. The effect should be the same as including DESTDIR in each install target directly.

@jvvv: I don't think USRSBINDIR is used anywhere anymore, but USRSHAREDIR is. Thank you for bringing it to my attention; I've just pushed a patch that should correctly define it.

@jcnelson
Copy link
Owner

Keeping this issue open, until we're sure that all the Makefiles correctly honor DESTDIR. Thank you all for your patience.

@jcnelson jcnelson reopened this Aug 30, 2015
@fbt
Copy link
Contributor

fbt commented Aug 30, 2015

So far so good. I've changed my PKGBUILD, it's now much easier to work with.
One thing: ETCDIR_VDEV could default to $(ETCDIR)/vdev

@jcnelson
Copy link
Owner

@fbt Just tweaked buildconf.mk to do so.

@fbt
Copy link
Contributor

fbt commented Aug 31, 2015

Sweet, thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants