povray 3.7.0.8 does not build on macOS 11.0 Big Sur

[fxcoudert]

Summary

povray 3.7.0.8 does not build on macOS 11.0 Big Sur

Environment

• Source code Git tag: 3.7.0.8
• Operating system: macOS 11.0.1
• Hardware platform: x86-64
• Compiler: clang 12 (system compiler)
• Regression from: macOS 10.15

Unix Pre-Build Output

Full prebuild output here: https://github.com/Homebrew/homebrew-core/runs/1402560071?check_suite_focus=true

Compiler/Linker Output

make install fails with:

In file included from lightgrp.cpp:35:
In file included from ./backend/frame.h:58:
In file included from ./backend/control/messagefactory.h:39:
In file included from ./backend/control/renderbackend.h:40:
In file included from ./backend/povray.h:46:
In file included from /usr/local/opt/boost/include/boost/thread.hpp:13:
In file included from /usr/local/opt/boost/include/boost/thread/thread.hpp:12:
In file included from /usr/local/opt/boost/include/boost/thread/thread_only.hpp:12:
In file included from /usr/local/opt/boost/include/boost/thread/detail/platform.hpp:14:
In file included from /usr/local/opt/boost/include/boost/config.hpp:44:
In file included from /usr/local/opt/boost/include/boost/config/detail/select_stdlib_config.hpp:24:
../version:1:1: error: expected unqualified-id
3.7.0.8
^

[wfpokorny]

I have no experience with macOS and I don't consider myself a real programmer.

With that disclaimer, here's my best guess at what is happening:

Guessing macOS is perhaps not a case sensitive OS?

Further supposing you are running a boost version of at least 1.73? This where boost starts to test for and use the c++20 headers if it finds it? (Wish the POV-Ray configure output the boost version it found instead of just that it passed the >= 'version' test. I couldn't tell what version you have with Big Sur (there is an 11.1 I saw) )

POV-Ray as an autoconf / gnu built program creates an upper case "VERSION" file in the root directory - this is a standard-ish thing.

The only way I think you can get the fail is if the c++ #include is finding the autoconf VERSION file instead of its own!

Perhaps my programming ignorance showing, but I thought the purpose of the '<' and '>' qualifier was to limit the header include search to c/c++ libraries? If so, no matter the case, how are we picking up the VERSION file in a parent directory? Is this a c++ pre-processor bug?

We could in prebuild.sh change VERSION to some other name, but then it's no longer really an autoconf standard set up. Is C++20 really trying force changes like this on who knows how many existing autoconf built projects?

Anyway still me doing a lot of guessing. To confirm we could try a couple things.

1. In your built environment and before testing, hack a copy of the prebuild.sh script to remove or rename the created VERSION file. It isn't necessary for the build.

2. I think the __has_include() macro test was added in c++14 or c++17 ??? It should be we could run configure with CXXFLAGS="-std=c++11" say and without the macro defined the actual #include won't happen.

One last note.

I'm running on Ubuntu 20.04.1 (g++ at c++14) and, with v3.8 at least, I'm NOT able to compile at c++17 or later as yet. No idea at the moment if this true for 3.7.0.8, but suspect it might be.

[fxcoudert]

Boost is 1.74.0, filesystem is case-insensitive. So your guess sounds right.

The VERSION file appears to be included because povray gives a lot of -I options adding to the search path:

clang++ -DHAVE_CONFIG_H -I. -I..  -I.. -I../source/backend -I../source/base -I../source/frontend -I../unix -I../vfe -I../vfe/unix -pthread -I/usr/local/opt/boost/include  -pipe -Wno-multichar -Wno-write-strings -O3 -ffast-math -march=native -D_THREAD_SAFE -c -o base/path.o base/path.cpp

[wfpokorny]

OK. So, in some testing just now I manually added an #include to a random .cpp file. It looks like the #include <> syntax doesn't limit the scope of directories searched to those shipped with c/c++ as I thought it did. (Why both <> and "" wrappers if not...)

Meaning if you have an include directory with a file name matching something in c++'s set, you include whatever is in that include directory file and not the c++ header file you really wanted.

Ref: https://en.cppreference.com/w/cpp/preprocessor/include

(Looks like any behaviour is allowed) Hmm, wonder if where the actual c++ version file exists, if it would always be found first?

Not sure what to do here for my own coding. Every easy work around feels like a kludge. Less messy changes look like a lot of work with potential downsides to day to day coding productivity. (I cannot myself fix what's in Github for POV-Ray code in any case)

The -I options as used by POV-Ray might be occasionally redundant, but none as looked at uniquely are extraneous given the code's current structure and build in place methodology. It's just what the Autotool's tools generate given where needed header files are found.

I'm pretty sure the trouble is the +i.. but that's needed to get to the config.h file in the parent directory from each sub directory where compiles take place.

Probably someone else has tripped on this already. I'll dig around when I next have some time.

[trevorsandy]

Boost somewhere after 1.60 breaks the POV-Ray build. I solved this by forcing brew to install Boost 1.60.

Pre-requisite:

• Download, edit and locally install the brew [email protected] resource bundle.
• Remark or remove this line:
  disable! because: :versioned_formula

Steps:

echo "--- Uninstall all versions of Boost ignoring dependencies and revert to 1.60..."
brew uninstall --force --ignore-dependencies boost
echo "--- Install [email protected] from local resource bundle..."
brew install ./[email protected]
echo "--- Force Boost 1.60 link and overwrite conflicting files..."
brew link --force --overwrite [email protected]

Cheers,

[wfpokorny]

@trevorsandy

I, just now, built 3.7.0.8 with boost 3.71 on my Ubuntu 20.04.1 machine, so what you're seeing for your compile is not universal.

(Changing the boost version however, might be another way to temporarily address the issue with picking up the VERSION file as the include supposing no dependency other than boost >=1.73 does this in the Big Sur environment)

Some thoughts. I know the uberpov branch, last updated in 2016 I think, needs updates related to newer boost versions. If you branched off something earlier than 3.7.0.8 (this the current 3.7-stable branch) - and haven't pulled and merged later updates - you'd likely have build issues too. Or it might be what you say is true for macOS or your set up.

[trevorsandy]

Perhaps the break occured after 1.60 - I didn't actually check at which version the break starts. What I do know is after brew updated Boost from 1.60 to 1.74 my builds broke.

Can you build with Boost 1.74 ?

Update: I'm building POV-Ray v3.8.0 source. This break is unique to macOS.

Cheers,

[wfpokorny]

@trevorsandy I don't know. I've not yet tried a build with anything beyond 1.71. Glad to hear you could build on other than macOS - it very likely means I can too.

Best regards,

[mitchblank]

Yes, the problem only occurs when building on a case-insensitive filesystem, which in practice means OS/X and potentially Windows (although I don't know if the windows build triggers this problem)

Just renaming the VERSION file to something like VERSION.txt in povray would be enough to solve the problem... the issue is just that #include <version> can accidentally find that VERSION file. Won't happen on a case-sensitive filesystem, of course.

[mitchblank]

I opened this homebrew PR to workaround the build issue there: Homebrew/homebrew-core#66356

[ryandesign]

POV-Ray as an autoconf / gnu built program creates an upper case "VERSION" file in the root directory - this is a standard-ish thing.

You must rename the VERSION file to something else. If having a file of this name is a standard, then the standard must be changed.

[mitchblank]

You must rename the VERSION file to something else. If having a file of this name is a standard, then the standard must be changed.

Well either that or keep the directories that have these VERSION files (., unix, libraries/tiff) from ending up in the include-path. That's why this is biting povray in particular -- the VERSION files are ending up places that the compiler is confusing them for header files.

[wfpokorny]

I've not found my way back to really looking at this, but my left ear is itching.

What I'm hoping is all compiler implementations have at some point implemented flag(s) to make the #include <> type search strict with respect to the source directory and file signatures for the standard c++ file names.

Otherwise, isn't this completely open include search methodology a security exposure?

Lots of c/c++ programmers around the planet. Forget case sensitivity, suppose someone is able to seed one or more directories on a computer with a file called "version". Imagine that file contains:

...
#include "./.snoop"
...

and that the hidden ".snoop" file was stashed somewhere on the system when "version" was seeded.

A new programmer sitting somewhere in Mexico writes a program with some headers in their home directory and compiles it with:

g++ MyFirstTryWithBoost.cpp -I.

a.out

[trevorsandy]

Exactly - you get it. This is why I reverted to Boost 1.60.

Cheers,

[mitchblank]

#include <version> was first introduced in Boost, but it was accepted as a standard header in C++20 -- https://en.cppreference.com/w/cpp/header/version

So just mandating older versions of boost isn't a long-term solution either; eventually that header will exist everywhere.

[wfpokorny]

OK. I had a little time today and I found a couple useful references - ones not of the screaming, why are you (using|not using) xyz, meanies variety. ;-)

No one, in what I found, was concerned with the security aspect of the open #include <> method for standard c/c++ headers. I see this as the real issue behind the initial issue. Forget about the particular version/VERSION name, autotools and POV-Ray.

Suppose somebody puts a few enticing projects out there who's real purpose is to seed writeable directories with landmine "version" files - or with other common-ish and commonly ignored names - and there will be programmers who will step on those land mines by using -I. or similar. I, often enough, write little test programs in whatever directory I happen to find myself. Who could resist trying out UltimaSuperExtraordinaryFastMathLib9 "... now gently massaging your tensed shoulders on every compile of your code ..."

Anyway...

The fix I plan to use:

The good news is that clang and g++ both support the -iquote flag. Use it over the -I. and -I..

This form searches the specified directories for quote style includes only. In simple testing, this approach fixes the original issue, plus it closes off most of the potential for accidentally including common library file names from other than "official" directories.

clang++ test.cpp -iquote.

g++ test.cpp -iquote.

Refs:

Bug 42540 - "version" include wreaks havoc on case-insensitive filesystems
https://bugs.llvm.org/show_bug.cgi?id=42540

Bug 236192 - C++20 poisoning
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236192

[wfpokorny]

An update. A few days ago started with the iquote update in my branch as being the best approach. It isn't completely working out.

The iquote approach appears to require the config.h include be specified relative to the directory location in which the file being compiled was read. A pain, but all these config.h paths can be updated. Do it and the source tree directory compiles work OK because the config.h file is in the correct relative positions.

Do a developer sort of thing and create build directories for compile variants. I do this by copying/linking the configure script to each of those variant directories; running configure with: ./configure --srcdir=... in each directory. What you find is that the compile is now broken because the unique config.h files gets created at the top of each build directory. In other words, config.h is no longer in the correct, relative to source file, location for iquote to work.

If I want to stick with the more secure Iquote approach with these variant directory builds, the only ugly solution I see is to place/replace each build's config.h at the top level of the distributed code base for each variation. I'm thinking this not really OK as it runs the potential to break users not aware of the non-standard workings.

I could do the VERSION name hack, but this doesn't address the -I. -I.. security issue. Plus there my original thought was to use Iquote for all non system include directories.

Another potential option would be to force all compiles to be completely outside the distributed code base in an empty directory when configure is run. I think this latter addresses the VERSION file problem - the distribution files don't exist at the top build level in that case. It addresses the security issues with -I. and -I.. only. Of course, the "in distribution" compile would no longer work as folks expect. I too have open questions like whether "make install" works as it should and I've not worked out how to discourage (prevent?) in code distribution compiles.

FWIW...

[wfpokorny]

OK. After thrashing about a long while trying stuff, my conclusion is there isn't any reasonably clean way to eliminate possible <> includes of files without file extensions. Not while being gnu/git compatible and without crippling some of the autotools developer functionality.

Who knows - smaller tools with non-recursive compiles perhaps will fair better. My little test case looked OK with -Iquote, but that didn't hold up. If you are just hacking to get the end config/compile in place to work, things can also be whatever compiles and runs.

What I did in the end was suffix all the gnu files with .txt in addition to eliminating file names without extensions in a reduced set of include directories. This requires autotools be run without the gnu/gnit restrictions. The default gnu setting: "The files INSTALL, NEWS, README, AUTHORS, and ChangeLog, plus one of COPYING.LIB, COPYING.LESSER or COPYING, are required at the topmost directory of the package."

Turns out VERSION, LICENSE(?) and THANKS (not currently a POV-Ray file) are required only by use of gnits or --gnits which does some conformance checking beyond the 'gnu' default.

On and off over the years there's been talk of POV-Ray becoming an official gnu utility, but it's not today - and my branch certainly isn't one. These gnu/gnit requirements can be relaxed. Using 'foreign,' the files aren't created for you or required for make dist/distcheck.

In unix/configure.ac change:

AM_INIT_AUTOMAKE([1.9 dist-bzip2 subdir-objects])

to

AM_INIT_AUTOMAKE([foreign 1.9 dist-bzip2 subdir-objects])

In the unix/prebuild.sh script in the section creating the 'bootstrap' script change:

automake --add-missing --warnings=all

to

automake --foreign --add-missing --warnings=all

I also explicitly set DEFAULT_INCLUDES for each Makefile.am to exclude the typically generated -I. leaving only the build directory include where config.h is created. This override generates warnings when automake is run for each of the compile subdirectories, but, it is what it is.

Those are the key changes. They drive a good many more, but should be those can be knocked down as seen. In testing everything I do developer wise is working as is the usual configure -> make -> make check - make install.

Aside: I think in this thread I mentioned c++17 compiles weren't working for me. They now are - in my branch at least. Unsure what changed, but I'll take the gift... :-)

[mitchblank]

For context on "gnits" see https://www.gnu.org/software/automake/manual/html_node/Strictness.html :

Unless you are a Gnits standards contributor, it is recommended that you avoid this option until such time as the Gnits standard is actually published (which may never happen).

Amusingly that comment "which may never happen" was added to the statement by @tromey back in 2001 which seems to have been prophetic since it's still there in 2021: https://git.savannah.gnu.org/cgit/automake.git/commit/?id=bea5cae85c9c534a76729d30cd8ef049333aee77

I also think the only added required file is THANKS. If you look at the automake code:

# Do any extra checking for GNITS standards.
sub check_gnits_standards ()
{
  if ($relative_dir eq '.')
    {
      # In top level (or only) directory.
      require_file ("$am_file.am", GNITS, 'THANKS');
    }
}

If you read the automake docs carefully it seems to imply that gnits requires that if VERSION exists it should have a certain format. So even in gnits mode it doesn't seem to require VERSION

Finally, I would point out that the automake tarball itself doesn't have a VERSION file.

Basically:

• I wouldn't worry about "gnits" compliance, since it doesn't seem to be a live concern
• Even in gnits mode, I think VERSION.txt or whatever is still allowed.

[wfpokorny]

@mitchblank Thanks for the additional insight. I'm not experienced with what's really getting used out and about autotools wise.

As for what gnits does by documentation at least, I ran across other bits which left me unsure what gnits was doing or not VERSION wise. The try-something -> build cycle <-- lookp while trying things was long an painful on my little two core.

For example, there is this bit: "The gnits option also implies options readme-alpha and check-news." Plus, it looks like these two options can by themselves be turned on apart from gnits itself; which left me wondering whether VERSION meant a file at all! Perhaps instead meaning only the VERSION variable in config.h ? "Version info" needs to exist somewhere always, if you can run check-news by itself.

Might it be the VERSION file has been a standard more or less by accident? Somebody created a VERSION file and most everyone just followed suit? In POV-Ray's build system it is used with AC_INIT.

AC_INIT([POV-Ray], m4_normalize(m4_include([VERSION.txt])),...)

Ref (just below green checked answer):
https://stackoverflow.com/questions/8559456/read-a-version-number-from-a-file-in-configure-ac

In any case, I agree, using VERSION.txt seems pretty safe - especially given you found it not to be forced to exist in the automake code.

[c-lipka]

I'm pretty sure there is some reason why the Unix build process would croak (on some machines, at any rate) without the ./VERSION file. But I doubt that file is actually a problem with regards to this issue.

Can someone test whether getting rid of the "original" - unix/VERSION - would solve the issue? The following change to the build process should do the trick:

run once:

mv unix/VERSION unix/VERSION.bak

to build:

cd unix
cp VERSION.bak VERSION
./prebuild.sh
rm VERSION
cd ..
./configure ...YaddaYaddaYadda...
...etc...

If that works, then I think the cleanest approach for all parties involved should be to just move the "original" of the VERSION file in a separate subdirectory where it won't be picked up.

While we're at it, we could do the same for all the other "originals" we're currently copying around verbatim or with modifications during the prebuild step; my suggestion would be unix/prebuild as the name for such a directory. The prebuild script itself would also be a candidate for movement. All in all, this would also make it easier to create dedicated Unix source code releases again, where the prebuild step has already been executed, and all clutter not required by the subsequent Unix build steps removed.

ATTENTION: One thing that would be important to look out for though, is that unix/VERSION may be used by tools and scripts beyond the scope of the Unix/Automake incarnation of POV-Ray. Such tools would have to be adapted, too.

(The good news is, POV-Ray v3.8 is unaffected by this issue nowadays, as it generates ./VERSION entirely on the fly, rather than copying it from a manually maintained "original".)

[c-lipka]

The prebuild script itself would also be a candidate for movement.

Then again, maybe let's not do that for v3.7. There are probably too many places where the file's location is explicitly documented as unix/prebuild,sh.

[c-lipka]

Please test whether branch hotfix/v370_issue403 solves the problem (and doesn't introduce any new ones).

[c-lipka]

Well, turns out v3.7 isn't the only one affected - v3.8 is suffering, too.

The crux of the matter is that configure (currently) automatically adds the root directory to the list of includes, because that's where it creates the config.h from all the information it gathers about the system.
Also, we've been forcing the addition of the root directory in some makefiles ourselves, for no immediately obvious reason.

So the solution (besides getting VERSION out of the unix directory, which also contains source code) appears to be to plonk config.h someplace else (and also not include the root directory ourselves anymore); I think unix/config.h sounds nice.

[trevorsandy]

So the solution...

Bravo! Welcome back @c-lipka.

Cheers,

[c-lipka]

Should be fixed in 3.7-stable branch (v3.7.0.9).
Corresponding fix in v3.8 still pending, but will follow same pattern.

[c-lipka]

Should be fixed in the next v3.8.0 release, too.