Doesn't Build on GCC4.9 Unless CFLAGS=-std=c99

[adamnovak]

I am running GCC4.9. This disagrees with some of the C code in HDF5, which uses C99-style // comments, but doesn't ask the compiler to compile it in C99 mode.

The tail end of my build from make looks like this:

th5s.c:733:9: error: expected expression before 
   ‘/’ token
         // ret = H5Pset_alloc_time(plist_id, alloc_time);
         ^
th5s.c:734:9: error: expected expression before 
   ‘/’ token
         // CHECK(ret, FAIL, "H5Pset_alloc_time");
         ^
In file included from th5s.c:25:0:
th5s.c: At top level:
H5srcdir.h:38:20: warning: ‘H5_get_srcdir_filename’
    defined but not used [-Wunused-function]
 static const char *H5_get_srcdir_filename(const char *filename)
                    ^
H5srcdir.h:68:20: warning: ‘H5_get_srcdir’
    defined but not used [-Wunused-function]
 static const char *H5_get_srcdir(void)
                    ^
make[3]: *** [th5s.o] Error 1
make[3]: Leaving directory `/hive/users/anovak/build/progressiveCactus/submodules/hdf5/test'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/hive/users/anovak/build/progressiveCactus/submodules/hdf5'
make[1]: *** [hdf5Rule] Error 2
make[1]: Leaving directory `/hive/users/anovak/build/progressiveCactus/submodules'
make: *** [all] Error 2
[anovak@kolossus progressiveCactus]$

Running CFLAGS=-std=c99 make builds successfully.

[czoller]

Should be CFLAGS=-std=c99 make with std instead of stc.

[adamnovak]

I've fixed it. I'm not sure how I managed to put "stc" in the issue.

Since actually putting stc in CFLAGS would probably break the build, I'm going to assume I knew what I was doing when I was actually setting the environment variable.

[adamnovak]

I can still reproduce this on the current Git version as of Friday. I tried to update, and I couldn't build without CFLAGS=-std=c99.

[andrewyatz]

Hi @adamnovak I've just tried to compile this with GCC 5.3 and found that the CFLAGS changes in the submodules Makefile fail to be used in HDF5's make. Did you manage to resolve the issue?

[adamnovak]

I don't recall having done so. I haven't built progressiveCactus again in over a year, and the workaround of just running CFLAGS=-std=c99 make got me the build I needed the last time I needed to build it. As far as I know, it might still require the variable set manually to build, or it might now build on 4.9 without that variable set.

[andrewyatz]

I think I found a partial solution. The compilation seems to have a problem with submodules/hdf5/tools/Makefile and those files have // comments in them. There's an odd additional script under config/gnu-flags in HDF5 that does a bit of CFLAGS editing (there are two flying around AM_CFLAGS and H5_CFLAGS) to replace instances of -ansi with -std=c99. The documentation is quite cryptic TBH concerning when to run this. It seemed easier to post-fix though after a lot of hacking I'm not too sure now.

I've hacked the submodules/Makefile to do a sed to do the replacement on the problematic Makefiles (tools/Makefile, tools/libs/Makefile, test/Makefile). We'll be publishing a lot of this binary building code soon (it's all in Linuxbrew)

[avapirev]

I can confirm the error persists in gcc 4.9, but compiles with no problems with gcc 4.8. I know that I will open a can of worms here, but using c++ comments-style in c code is not a good practise. For some reason I am not able to force the build to use CFLAGS=-std=c99 exactly for what the previous comment says - the -ansi flag is forced on AM_CFLAGS and H5_CFLAGS no matter what. The solution: I also had to resort to the good old seek-and-replace script with sed for all *.c files.

[andrewyatz]

@avapirev so you had not joy if you edit the makefiles directly? I agree the c++ style comments isn't great but that's not really a fault of cactus but HDF5.

Perhaps the most baffling for me is that https://github.com/Linuxbrew/homebrew-science/blob/f70a4e5772b40c3c80136ef35c8888e65af2f0eb/hdf5.rb is the homebrew-science version of compiling hdf5 and that has no need for hacks. All I can think is that there's been a change in the way hdf5 compiles that might be solved here with an upgrade of hdf5 version. But that's a big ask for a possible answer

[glennhickey]

Yeah, I'd imagine hdf5 has long since been patched for this. @joelarmstrong and company are completely reworking the packaging of cactus right now, if I understand correctly, so it'd be a good opportunity to try bringing hdf5 up to date..

On Wed, Oct 26, 2016 at 10:19 AM, Andrew Yates notifications@github.com wrote:

@avapirev https://github.com/avapirev so you had not joy if you edit the makefiles directly? I agree the c++ style comments isn't great but that's not really a fault of cactus but HDF5.

Perhaps the most baffling for me is that https://github.com/Linuxbrew/ homebrew-science/blob/f70a4e5772b40c3c80136ef35c8888e65af2f0eb/hdf5.rb is the homebrew-science version of compiling hdf5 and that has no need for hacks. All I can think is that there's been a change in the way hdf5 compiles that might be solved here with an upgrade of hdf5 version. But that's a big ask for a possible answer

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/glennhickey/progressiveCactus/issues/25#issuecomment-256361813, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2_7l8G3Y2Bs_fkHbth7JR3Fk0TCbgnks5q32F5gaJpZM4CaVqT .

[andrewyatz]

That would be pretty awesome. Any ideas on timelines and what form this repackaging will take?

On 26 October 2016 at 16:45, Glenn Hickey notifications@github.com wrote:

Yeah, I'd imagine hdf5 has long since been patched for this. @joelarmstrong and company are completely reworking the packaging of cactus right now, if I understand correctly, so it'd be a good opportunity to try bringing hdf5 up to date..

On Wed, Oct 26, 2016 at 10:19 AM, Andrew Yates notifications@github.com wrote:

@avapirev https://github.com/avapirev so you had not joy if you edit the makefiles directly? I agree the c++ style comments isn't great but that's not really a fault of cactus but HDF5.

Perhaps the most baffling for me is that https://github.com/Linuxbrew/ homebrew-science/blob/f70a4e5772b40c3c80136ef35c8888e65af2f0eb/hdf5.rb is the homebrew-science version of compiling hdf5 and that has no need for hacks. All I can think is that there's been a change in the way hdf5 compiles that might be solved here with an upgrade of hdf5 version. But that's a big ask for a possible answer

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/glennhickey/progressiveCactus/ issues/25#issuecomment-256361813, or mute the thread https://github.com/notifications/unsubscribe-auth/AA2_7l8G3Y2Bs_ fkHbth7JR3Fk0TCbgnks5q32F5gaJpZM4CaVqT

.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/glennhickey/progressiveCactus/issues/25#issuecomment-256389486, or mute the thread https://github.com/notifications/unsubscribe-auth/AAK5GYXCM4V4D99saHdTQ5a2RKR7dfX9ks5q33WYgaJpZM4CaVqT .

[avapirev]

@andrewyatz I played a bit with the build system but in general I prefer to not make any changes to it. It took few minutes to sed-grep-etc... the bad comments. Changing the build might result in performance and accuracy issues at the least. The code developers have done it that way for one reason, or another, and since I don't know those reasons I consider it better to leave the build alone in peace. Also, I have never had any problems compiling HDF5 on all sorts of systems with all sorts of tools. Again, if there is a local copy of HDF5 in the ProgressiveCactus suite probably there is a good reason for that (I hope). If not, then the devepers probably should mark HDF5 and all other side-dish libs as necessary tools. That would be, e.g., load them as modules, install on your own with properly exported paths, etc... Cheers.