TritonDataCenter / smartos-live

For more information, please see http://smartos.org/ For any questions that aren't answered there, please join the SmartOS discussion list: https://smartos.topicbox.com/groups/smartos-discuss
1.58k stars 247 forks source link

gmake clean on smartos-live root doesn't full clean everything #736

Open nerakhon opened 7 years ago

nerakhon commented 7 years ago

I hit this issue during my first build of smartos. When I initially failed to set up large enough quota for smartos build on my buildzone the build failed. So I resized the quota did a gmake clean and restarted the build. The build kept failing on installing iconv library into protoarea due to missing 64 bit version:

install: cp amd64/646%CODESET.so /smartos-live/projects/illumos-extra/g11n/proto/i386/fileroot/usr/lib/iconv/amd64/646%CODESET.so failed

I kept investigating and found out that the issue is that the build of g11n from the extras repo skipped building this directory because it found a partially built binaries.

So I am not sure what is the better solution but IMO I would expect gmake clean to really go through all the cloned repositories and get rid of all the built artefacts including these half built ones.

rmustacc commented 7 years ago

Thanks for reporting this. Taking a look, the default clean target in the smartos-live Makefile does recur into illumos-extra and run gmake clean. See this snippet from the top-level Makefile:

363         [ ! -d $(ROOT)/projects/illumos-extra ] || \
364             (cd $(ROOT)/projects/illumos-extra && gmake clean)

Now, the gmake clean target in illumos-extra does end up going through and running it in the g11n directory, which itself invokes gmake clean.

Now, I suspect this is where things go awry. The g11n makefile structure inherits some Sun-isms. Importantly it has both a clean and a clobber target. The clean target only removes intermediate files, but leaves generated artifacts. This is likely why this particular failure mode occurred. I think the solution will be to change the top-level Makefile in g11n to invoke the clobber target instead of the clean target. I'll put something together to test this theory and report back.

rmustacc commented 7 years ago

There's one other thing I discovered, because of the use of subshells it seems like if we don't find dmake, it'll silently fail.

rmustacc commented 7 years ago

I've put together a review at https://cr.joyent.us/#/c/2301/.

nerakhon commented 7 years ago

Coming myself from Sun/Oracle Solaris organisation originally I was surprised to see clobber missing in illumos :). Thx for looking into it! And for keeping SunOS alive, I'm glad to be finally outside Larry's world now and still able to use all the technologies I fell in love with.

nerakhon commented 6 years ago

Do I get it right from the gerrit review that this was already merged and fixed?

rmustacc commented 6 years ago

@nerakhon Yeah, this should have been fixed back in August. Sorry, I forgot to close this. Hopefully you're not seeing this as still a problem?