omnios-build
omnios-build copied to clipboard
omniti-labs
fix NOSCRIPTSTUB
Enabling NOSCRIPTSTUB just disables stubbing altogether on master. Merging this should fix that.
In addition I think enabling NOSCRIPTSTUB should mv files to bin from the arch directories instead of copying them, or you end up having two copies of each script in arch directories which are not likely to be run ever.
The first change looks fine as a fix for not getting ELF stubs when NOSCRIPTSTUB is non-empty. In thinking about the second change (cp vs. mv) there is actually a deeper problem. If the non-ELF files in i386 vs. amd64 are different, then neither copying nor moving one of them up to the bin/sbin directory is correct.
As an example, see /usr/bin/{i386,amd64}/xml2-config. xml2-config is a shell script, but the content differs because each returns arch-specific build information. Generally speaking, one should explicitly call /usr/bin/i386/xml2-config when used in a 32-bit build, but that may not be possible with every piece of software that wants to run xml2-config. In that case, maybe we do want to create a stub so that I can, for example, do 'ISALIST=i386 ./configure ...' and know that the right version will get run.
Using mv instead of cp is clearly better, but I think the logic should be extended such that if NOSCRIPTSTUB is set:
If the file is non-ELF, and there is either only one copy (BUILDARCH is not "both") or the copies are identical, mv the file to the upper directory.
OR
If the file is non-ELF, but the arch versions differ, make a stub anyway (or possibly do nothing, I'm not sure which is better)
Regardless, I think it's good to fix this, though it is rarely used. Nothing in the core build uses it, and only two things in omniti-ms do. Of those two, one is 32-bit only and therefore doesn't have a problem, but the other is dual-arch and hits the no-stubs-for-binaries issue.
On Thu, Apr 11 2013 11:38:53 -0700, esproul wrote:
If the file is non-ELF, but the arch versions differ, make a stub anyway (or possibly do nothing, I'm not sure which is better)
I'd say creating the stub is better here, otherwise anyone who installs the package won't be able to run the non-ELF very easily.
Lauri Tirkkonen | +358 50 5341376 | lotheac @ IRCnet
Hmmm. This is two commits (one for the read(1) fix, and one for the s/cp/mv/). Is that your intention, or do you want them as one commit?
Also, do you care if I push these manually (with your name, etc.) in instead of merging the pull request? Less merge turds if I do the former.
On Thu, May 22 2014 12:33:08 -0700, Dan McDonald wrote:
Hmmm. This is two commits (one for the read(1) fix, and one for the s/cp/mv/). Is that your intention, or do you want them as one commit?
It's your repo, you get to decide what the history looks like. I will squash them if you want.
Also, do you care if I push these manually (with your name, etc.) in instead of merging the pull request? Less merge turds if I do the former.
Doesn't matter terribly much to me either way, but I don't know what you mean by merge turds.
Lauri Tirkkonen | +358 50 5341376 | lotheac @ IRCnet
Yes, in use in our repo niksula/omnios-build for a long time (eg. https://github.com/niksula/omnios-build/blob/master/build/puppet/build.sh)
Having said that I should probably implement the additional logic from esproul's comment above before this is merged.