At the moment, config.guess returns a string that looks like "i386-pc-os2_emx" or "i386-pc-os2_vacpp". A while back, while talking to one of the config.* developers, I was informed that it is not standard to include any refernce to the compiler in the string. This needs to be changed to match other platforms.
Created attachment 59382 [details] [diff] [review] patch I changed to using the generic string "i386-pc-os2" and determining the difference between an EMX/GCC and VisualAge C++ build by testing the $CC variable. I am also going to submit the config.* patches to the maintainers to get their input.
Why not test against GNU_CC rather than explicitly checking for icc? Your tests will break if anyone tries to pass CC=/full/path/icc to configure.
GNU_CC seems to only be set if the environment variable GCC is set. While EMX/GCC may set this, it doesn't help with compilers that are not gcc. In our case, I decided to look at the value of CC. You are right, though, that this may be a full path. Should we require developers to set an environment variable (such as VACPP = yes) and then check for that in configure.in?
Created attachment 59415 [details] [diff] [review] another patch OK, in this version of configure.in, I check for $GNU_CC and $VACPP. A user of EMX/GCC would set "$GCC" = "yes", while a user of Visual Age would set "$VACPP" = "yes". Also, I have taken the OS/2 specific changes from the current versions of config.guess and config.sub. How does this look?
Attachment #59382 - Attachment is obsolete: true
Testing for "$GNU_CC" should work when looking for non-gcc compilers as well unless you're supporting multiple non-gcc compilers. I didn't think that was the case here. I'd avoid making people explicitly set VACPP if possible.
Well, I thought that assuming that any non-gcc compiler is VACPP would be wrong; that it would be best to explicitly check for the compiler. Secondly, we already set VACPP=yes. In fact, what we do have to change is to set GCC=yes for EMX/GCC.
Comment on attachment 59415 [details] [diff] [review] another patch Ok, if you already require VACPP to be set, then the patch is fine as is. r=cls Btw, configure automatically sets GCC=yes if it detects that $CC is a gcc variant so there's no need to tell the developer to do so.
Attachment #59415 - Flags: review+
Target Milestone: --- → mozilla0.9.7
Patch has been checked into the mozilla trunk, NSPR trunk and NSPR NSPRPUB_PRE_4_2_CLIENT_BRANCH branch.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.