{"thread":{"id":"29430","subject":"make install rewrites source files","startedAt":"2012-01-23T14:18:18Z","lastAt":"2012-01-27T13:11:46Z","messageCount":9,"participants":["Hallvard Breien Furuseth","Junio C Hamano","Phillip Susi","Clemens Buchacher","Hallvard B Furuseth"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"182964","messageId":"hbf.20120123bz2f@bombur.uio.no","threadId":"29430","inReplyTo":null,"subject":"make install rewrites source files","fromName":"Hallvard Breien Furuseth","fromEmail":"h.b.furuseth@usit.uio.no","sentAt":"2012-01-23T14:18:18Z","receivedAt":"2012-01-23T14:18:18Z","isPatch":false,"sender":{"key":"h.b.furuseth@usit.uio.no","avatar":null},"body":"INSTALL says we can install a profiled Git with\n\t$ make profile-all\n\t# make install prefix=...\nThis does not work: 'make install' notices that the build flags has\nchanged and rebuilds Git - presumably without using the profile info.\nThe patch below fixes this.\n\nHowever, make install should not write to the source directory in any\ncase.  That fails as root if root lacks write access there, due to NFS\nmounts that map root to nobody etc.  At least git-instaweb and\nGIT-BUILD-OPTIONS are rewritten.  You can simulate this with\n    su nobody -s /bin/bash -c 'make -k install'\nafter configuring with prefix=<directory owned by nobody>.\n\n\nIndex: INSTALL\n--- INSTALL~\n+++ INSTALL\n@@ -29,6 +29,6 @@ If you're willing to trade off (much) lo\n faster git you can also do a profile feedback build with\n \n-\t$ make profile-all\n-\t# make prefix=... install\n+\t$ make profile-all     prefix=...\n+\t# make profile-install prefix=...\n \n This will run the complete test suite as training workload and then\nIndex: Makefile\n--- Makefile~\t2012-01-19 01:36:02.000000000 +0100\n+++ Makefile\t2012-01-23 14:44:56.554980323 +0100\n@@ -2695,5 +2695,5 @@ cover_db_html: cover_db\n ### profile feedback build\n #\n-.PHONY: profile-all profile-clean\n+.PHONY: profile-all profile-clean profile-install\n \n PROFILE_GEN_CFLAGS := $(CFLAGS) -fprofile-generate -DNO_NORETURN=1\n@@ -2708,2 +2708,5 @@ profile-all: profile-clean\n \t$(MAKE) CFLAGS=\"$(PROFILE_GEN_CFLAGS)\" -j1 test\n \t$(MAKE) CFLAGS=\"$(PROFILE_USE_CFLAGS)\" all\n+\n+profile-install:\n+\t$(MAKE) CFLAGS=\"$(PROFILE_USE_CFLAGS)\" install\n"},{"id":"182980","messageId":"7vhazm89bo.fsf@alter.siamese.dyndns.org","threadId":"29430","inReplyTo":"hbf.20120123bz2f@bombur.uio.no","subject":"Re: make install rewrites source files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-23T20:15:07Z","receivedAt":"2012-01-23T20:15:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no> writes:\n\n> INSTALL says we can install a profiled Git with\n> \t$ make profile-all\n> \t# make install prefix=...\n> This does not work...\n\nWe should just drop prefix=... from that line, as the \"prefix=value must\nbe the same while building and installing\" is not only about the \"profile\"\nbuild but applies to any other build.\n\nI however wonder why you would need a separate profile-install target,\nthough.  Shouldn't \n\n\t$ make foo-build && make install\n\ninstall a funky 'foo' variant of the build for any supported value of\n'foo'?\n"},{"id":"182981","messageId":"4F1DC2F7.2070502@ubuntu.com","threadId":"29430","inReplyTo":"hbf.20120123bz2f@bombur.uio.no","subject":"Re: make install rewrites source files","fromName":"Phillip Susi","fromEmail":"psusi@ubuntu.com","sentAt":"2012-01-23T20:28:39Z","receivedAt":"2012-01-23T20:28:39Z","isPatch":false,"sender":{"key":"psusi@ubuntu.com","avatar":null},"body":"-----BEGIN PGP SIGNED MESSAGE-----\nHash: SHA1\n\nOn 1/23/2012 9:18 AM, Hallvard Breien Furuseth wrote:\n> INSTALL says we can install a profiled Git with $ make profile-all \n> # make install prefix=...\n\nprefix should be an argument to configure, not make.\n\n> This does not work: 'make install' notices that the build flags\n> has changed and rebuilds Git - presumably without using the profile\n> info. The patch below fixes this.\n\nmake install implicitly includes make all; it is supposed to rebuild\nanything that needs rebuilt.\n\n> However, make install should not write to the source directory in\n> any case.  That fails as root if root lacks write access there, due\n> to NFS mounts that map root to nobody etc.  At least git-instaweb\n> and GIT-BUILD-OPTIONS are rewritten.  You can simulate this with su\n> nobody -s /bin/bash -c 'make -k install' after configuring with\n> prefix=<directory owned by nobody>.\n\nIf you want to build locally from a read only nfs mount, then you\nshould run the configure script in a local directory:\n\nmkdir /tmp/build\ncd /tmp/build\n/path/to/nfs/source/configure\nmake\nmake install\n\n> Index: Makefile --- Makefile~\t2012-01-19 01:36:02.000000000 +0100 \n> +++ Makefile\t2012-01-23 14:44:56.554980323 +0100\n\nHrm... Makefile should itself be a generated file from Makefile.in or\nMakefile.am, but it appears that git isn't doing this.  Perhaps that\nshould be fixed.\n\n-----BEGIN PGP SIGNATURE-----\nVersion: GnuPG v2.0.17 (MingW32)\nComment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/\n\niQEcBAEBAgAGBQJPHcL3AAoJEJrBOlT6nu759U8IANCtJDWnCizSDWrJAFWe3ISr\nFemiFgW347qjLcWlJS036nPfKnrxrJ88rF2e9+8Tj/hfPojNwCmyvN7rz+guI0uA\nqqOfk9uN38Qd/jwfW5gv/7raKP4eUyRZ9ioptX3NqQtP5Co4TFuajOfswpN8f/DL\nQiU7os62Df5HWW2U8A3XT9KiU9oWRala8dcrp5EJkEOfYDvQG2o3e1N/D91KC4el\nlAyVEnzrvoLr5NzHCnFe7dQqvAB2S3PE/NP4anZHyNRp3SDLu1iZbD9MKC21Bd3n\nBmCn9Vh7U+reC/NBMq8qaM69jLRk2Dx12brFoyY5/cjdQuaLj+n6h1nN9MqSKYI=\n=/qLy\n-----END PGP SIGNATURE-----\n"},{"id":"182982","messageId":"7vaa5e87lt.fsf@alter.siamese.dyndns.org","threadId":"29430","inReplyTo":"4F1DC2F7.2070502@ubuntu.com","subject":"Re: make install rewrites source files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-23T20:52:14Z","receivedAt":"2012-01-23T20:52:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Susi <psusi@ubuntu.com> writes:\n\n> On 1/23/2012 9:18 AM, Hallvard Breien Furuseth wrote:\n>> INSTALL says we can install a profiled Git with $ make profile-all \n>> # make install prefix=...\n>\n> prefix should be an argument to configure, not make.\n\nWhat are you talking about?  Use of ./configure is entirely optional, and\nour Makefile _does_ support giving prefix on the command line.\n"},{"id":"182983","messageId":"hbf.20120123j61g@bombur.uio.no","threadId":"29430","inReplyTo":"7vhazm89bo.fsf@alter.siamese.dyndns.org","subject":"Re: make install rewrites source files","fromName":"Hallvard Breien Furuseth","fromEmail":"h.b.furuseth@usit.uio.no","sentAt":"2012-01-23T20:57:51Z","receivedAt":"2012-01-23T20:57:51Z","isPatch":false,"sender":{"key":"h.b.furuseth@usit.uio.no","avatar":null},"body":"On Mon, 23 Jan 2012 12:15:07 -0800, Junio C Hamano <gitster@pobox.com> wrote:\n> Hallvard Breien Furuseth <h.b.furuseth@usit.uio.no> writes:\n> \n>> INSTALL says we can install a profiled Git with\n>> \t   $ make profile-all\n>> \t   # make install prefix=...\n>> This does not work...\n> \n> We should just drop prefix=... from that line, as the \"prefix=value must\n> be the same while building and installing\" is not only about the \"profile\"\n> build but applies to any other build.\n\nEither add or remove a prefix so they match, yes.  Fine by me either way.\n\n> I however wonder why you would need a separate profile-install target,\n> though.  Shouldn't \n> \n>\t$ make foo-build && make install\n> \n> install a funky 'foo' variant of the build for any supported value of\n> 'foo'?\n\n'profile-all' makes 'all' with different CFLAGS from those in\nMakefile.  'install' makes 'all' which notices CFLAGS has changed\nsince last build, so it rebuilds:\n    $ make install\n    * new build flags or prefix\n    ...\nThat's also how the 2nd '$(MAKE) ... all' in profile-all can tell\nthat it should do anything.  Thus my new 'profile-install:' target\nwith the same flags as the final $(MAKE) in profile-all.\n\nThis looks way too clever to me.  'make' can detect that flags have\nchanged, but should then fail (optionally?) instead of rebuilding.\nThat'd likely solve my issue with other files rewritten as root too.\nBut I'm not volunteering to rewrite your build system.\n\nBTW, it'd be useful to split up 'profile-all' so it is possible\nto ignore 'make test' failure and compilete the build anyway:\n\n.PHONY: profile-all profile-clean profile-gen profile-use profile-install\nprofile-all: profile-clean profile-gen profile-use\nprofile-gen:\n\t$(MAKE) CFLAGS=\"$(PROFILE_GEN_CFLAGS)\" all\n\t$(MAKE) CFLAGS=\"$(PROFILE_GEN_CFLAGS)\" -j1 test\nprofile-use:\n\t$(MAKE) CFLAGS=\"$(PROFILE_USE_CFLAGS)\" all\n\n-- \nHallvard\n"},{"id":"183141","messageId":"20120126225231.GA14753@ecki","threadId":"29430","inReplyTo":"hbf.20120123j61g@bombur.uio.no","subject":"Re: make install rewrites source files","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-01-26T22:52:31Z","receivedAt":"2012-01-26T22:52:31Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Mon, Jan 23, 2012 at 09:57:51PM +0100, Hallvard Breien Furuseth wrote:\n> \n> 'profile-all' makes 'all' with different CFLAGS from those in\n> Makefile.\n\nHow about removing the profile-all target and making it a build option\ninstead? To enable it, do the usual:\n\n echo PROFILE_BUILD=YesPlease >> config.mak\n echo prefix=... >> config.mak\n make\n su make install\n\nIn the Makefile, we would have\n\nifdef PROFILE_BUILD\n all:\n\t$(MAKE) CFLAGS=... -fprofile-generate ... all-one\n\t$(MAKE) CFLAGS=... -fprofile-use ... all-one\nelse\n all: all-one\nendif\n\nand each previous instance of 'all' replaced with 'all-one'.\n\nClemens\n"},{"id":"183151","messageId":"7vobtq0y1j.fsf@alter.siamese.dyndns.org","threadId":"29430","inReplyTo":"20120126225231.GA14753@ecki","subject":"Re: make install rewrites source files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-27T00:49:44Z","receivedAt":"2012-01-27T00:49:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> How about removing the profile-all target and making it a build option\n> instead? To enable it, do the usual:\n>\n>  echo PROFILE_BUILD=YesPlease >> config.mak\n>  echo prefix=... >> config.mak\n>  make\n>  su make install\n\nYeah, I would prefer something like that. We could even keep \"profile-all\"\ntarget for b/c if we wanted to, no?\n"},{"id":"183181","messageId":"79b0e5e55a438cc757cfeb6408be4d23@ulrik.uio.no","threadId":"29430","inReplyTo":"4F1DC2F7.2070502@ubuntu.com","subject":"Re: make install rewrites source files","fromName":"Hallvard B Furuseth","fromEmail":"h.b.furuseth@usit.uio.no","sentAt":"2012-01-27T09:46:55Z","receivedAt":"2012-01-27T09:46:55Z","isPatch":false,"sender":{"key":"h.b.furuseth@usit.uio.no","avatar":null},"body":" On Mon, 23 Jan 2012 15:28:39 -0500, Phillip Susi <psusi@ubuntu.com> \n wrote:\n> On 1/23/2012 9:18 AM, Hallvard Breien Furuseth wrote:\n>> However, make install should not write to the source directory in\n>> any case.  That fails as root if root lacks write access there, due\n>> to NFS mounts that map root to nobody etc.  At least git-instaweb\n>> and GIT-BUILD-OPTIONS are rewritten.  You can simulate this with su\n>> nobody -s /bin/bash -c 'make -k install' after configuring with\n>> prefix=<directory owned by nobody>.\n>\n> If you want to build locally from a read only nfs mount, then you\n> should run the configure script in a local directory:\n>\n> mkdir /tmp/build\n> (...)\n\n Not a read-only nfs mount.  Just an ordinary remote mount where root\n on the local host is mapped to nobody on the remote host.  (Having\n local root access does not mean you should get root on the remote.)\n\n In any case, it's normal practice to do as little as possible as root,\n and also to at least try not write to the source dir during install.\n BTW, building in /tmp can be nasty to other users when you don't know\n how much space the build (and maybe test) will use, so you may need\n access to some other local dir.\n\n-- \n Hallvard\n"},{"id":"183185","messageId":"hbf.20120127mhkz@bombur.uio.no","threadId":"29430","inReplyTo":"20120126225231.GA14753@ecki","subject":"Re: make install rewrites source files","fromName":"Hallvard Breien Furuseth","fromEmail":"h.b.furuseth@usit.uio.no","sentAt":"2012-01-27T13:11:46Z","receivedAt":"2012-01-27T13:11:46Z","isPatch":false,"sender":{"key":"h.b.furuseth@usit.uio.no","avatar":null},"body":"On Thu, 26 Jan 2012 23:52:31 +0100, Clemens Buchacher <drizzd@aon.at> wrote:\n> How about removing the profile-all target and making it a build option\n> instead? To enable it, do the usual:\n> (...)\n> ifdef PROFILE_BUILD\n>  all:\n> \t$(MAKE) CFLAGS=... -fprofile-generate ... all-one\n> \t$(MAKE) CFLAGS=... -fprofile-use ... all-one\n> else\n>  all: all-one\n> endif\n> \n> and each previous instance of 'all' replaced with 'all-one'.\n\nNot quite.  test: and install: should depend on 'all', otherwise making\nthem without doing 'make all' first will test/install an unprofiled Git.\n\nSo 'all' with profiling should be today's profile-all, which should not\nthrow away the build and start over.  It can create some files to mark\nhow far it has gotten instead.  And profile-generate currently uses\n'test' which would recurse, it needs another internal test target.\n\nNot sure if it is worth it.  Something like this, perhaps.  Except I\nhave not thought about how this interacts with the coverage targets.\n\n# Final targets\n\nifdef PROFILE_BUILD\nall::\t\tprofile-all\ntest:\t\tprofile-test\ninstall:\tprofile-install\nelse\nall::\t\tall-one\ntest:\t\ttest-one\ninstall:\tinstall-one\nendif\n\n# Profiling\n#\n# Note: If profiling (the test phase) failed halfway through but you\n# still want to use the partial profile results to build Git, you can\n#\ttouch p-gen.stamp\n# and then 'make all' again.\n\nprofile-all: p-use.stamp\n\nprofile-gen p-gen.stamp:\n\t$(MAKE) CFLAGS=\"$(PROFILE_GEN_CFLAGS)\" all-one\n\t$(MAKE) CFLAGS=\"$(PROFILE_GEN_CFLAGS)\" -j1 test-one\n\ttouch p-gen.stamp\n\nprofile-use p-use.stamp: p-gen.stamp\n\t$(MAKE) CFLAGS=\"$(PROFILE_USE_CFLAGS)\" all-one\n\ttouch p-use.stamp\n\nprofile-test: p-use.stamp\n\t$(MAKE) CFLAGS=\"$(PROFILE_USE_CFLAGS)\" test-one\n\nprofile-install: p-use.stamp\n\t$(MAKE) CFLAGS=\"$(PROFILE_USE_CFLAGS)\" install-one\n\n.PHONY: all-one test test-one install install-one\n.PHONY: profile-all profile-gen profile-test profile-install profile-clean\n\n\nAlso let 'clean' depend on 'profile-clean' which does\n\t$(RM) p-gen.stamp p-use.stamp.\n\n-- \nHallvard\n"}]}