View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002098ThirdPartyPatchpublic2016-05-24 20:472016-05-30 21:26
Reporterprojectionist Assigned Tohenry  
PrioritylowSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformLinuxOSArch LinuxOS Version4.5.4-1-ARCH
Fixed in Versiondev 
Summary0002098: ParaView fails to build with gcc-6
DescriptionIf I try to compile ParaView on my system (Arch Linux with gcc-6.1.1), the compilation quickly fails. I narrowed the reason down to two files:

ParaView-5.0.0/VTK/CMake/vtkCompilerExtras.cmake
ParaView-5.0.0/VTK/CMake/GenerateExportHeader.cmake

In this files, the version of gcc is determined by regular expression from gcc's --version output.
However, the regular expression is written in a way to fail for gcc > 6.0


The offending line of code in both files is:

string(REGEX MATCH "[345]\\.[0-9]\\.[0-9]*"

by replacing, this line with:

string(REGEX MATCH "[3456]\\.[0-9]\\.[0-9]*"

I am able to proceed in the building process.
Additional InformationBug has been reported upstream to ParaView bug tracker
http://www.paraview.org/Bug/view.php?id=16151

I attached the necessary patches to this report. These were created with the sources of ParaView-5.0.0. On other versions, this bug is also present, however, on different lines (e.g. vtkCompilerExtras.cmake of ParaView-4.1.0).
TagsNo tags attached.

Activities

projectionist

2016-05-24 20:47

reporter  

paraView_gcc6_patches.tar.gz (252 bytes)

wyldckat

2016-05-24 23:27

updater   ~0006316

Sorry for the confusion with the project+category changes, but I had tried to move this report to the ThirdParty project and for a few minutes I thought I had broken MantisBT, because the "Patch" category doesn't exist in the ThirdParty project here at the bug tracker... which lead to a big confusion...

Anyway, I've finally managed to move this issue to the ThirdParty project, given that it's the correct location for this report.


@projectionist: Many thanks for the report and patch! This should come in handy when the next release of OpenFOAM is done.

Nonetheless, for the repository-based version, the README.org file is usually the place where the fix is given at, for which the following command should do the trick:

  sed -ibck -e 's/\[345\]/[3456]/' ParaView-5.0.0/VTK/CMake/{GenerateExportHeader,vtkCompilerExtras}.cmake


I've checked these files and this issue is also present in ParaView 5.0.1.

henry

2016-05-25 08:41

manager   ~0006317

@Bruno: I have added "Contribution" and "Patch" categories to the ThirdParty-dev project which should avoid further confusion.

Given that we usually need to patch ParaView sources in order to build it on a range of hardware it might be a good idea to include the patched ParaView in the ThirdParty-dev repository for convenience -- thoughts?

wyldckat

2016-05-25 13:09

updater   ~0006318

@Henry: Many thanks for adding the missing categories! Adding "dev" to the "product version" list might also come in handy ;)


OK, I guess we'll be entering the "patching hell" phase... but I guess it's better than having a document that lists all of the fixes that need to be done.


So far we have 2 types of fixes needed for ParaView:

  1. Those that can be done with relying on "sed".

  2. Those that have to be done by replacing a file with one from the upstream source code.

The third type will show up when we need to rely on the utility "patch" to apply a patch file. If and when this time comes, the patch files can be stored at "etc/patches", based on the version number.

As for patching in general, this can be handled with a shell function "applyPatches()" at "etc/tools/ParaViewFunctions", which should be called from within "configParaView()" (near its start).

The function "applyPatches()" should check for (and create if not present) a tag file e.g. "patched.by.makeParaView" and then to apply the necessary fixes (if the tag didn't exist yet).

henry

2016-05-25 13:32

manager   ~0006319

>Adding "dev" to the "product version" list might also come in handy

Done.

What I am suggesting is to include the complete ParaView source tree in the ThirdParty-dev repository with all the patches and tweaks already applied so people only need to clone/pull and compile.

wyldckat

2016-05-25 14:20

updater   ~0006320

Last edited: 2016-05-25 14:22

 Sorry, I did misread what you had written.

Then it's possibly best to rely on git submodules, so that the "ThirdParty-dev" repository isn't flooded with a lot of source code from ParaView.

I've got some experience on this... First fork this repository: https://github.com/Kitware/ParaView/ - into github.com/OpenFOAM

Then locally, do the following commands in ThirdParty-dev:

  git submodule add https://github.com/OpenFOAM/ParaView.git ParaView-5.0.1
  git submodule init
  git submodule update

  git add .gitmodules
  git commit -m "Added new submodule for ParaView-5.0.1."


You can have more submodule folders that rely on the same project, something like this:

  git submodule add https://github.com/OpenFOAM/ParaView.git ParaView-4.4.0
  git submodule init
  git submodule update

  git add .gitmodules
  git commit -m "Added new submodule for ParaView-4.4.0."


The submodule feature will then index to the commit you are using within each folder. For example:

  cd ParaView-5.0.1
  git checkout v5.0.1
  git checkout -b version-5.0.1

  # apply here the patches

  cd ..
  git add ParaView-5.0.1
  git commit -m "Synced with ParaView 5.0.1."


This will make the commit at ThirdParty-dev reference the commit hash that is being used inside the folder "ParaView-5.0.1".

The downside of using git submodules is that this will tag the commit on the submodule and it will not care about which branch its on. This means that we need to be careful on a new checkout.


For example, people who clone from https://github.com/OpenFOAM/ThirdParty-dev/ - will have to do the following steps:

   git clone https://github.com/OpenFOAM/ThirdParty-dev.git

   # and if it didn't do the submodule init+update automatically (I vaguely remember this depends on the version of Git):

   cd ThirdParty-dev
   git submodule init
   git submodule update


Now, if people want to contribute patches, they will need to do the following, e.g.:

  cd ParaView-5.0.1
  git checkout version-5.0.1

so that the changes are committed to the correct branch and not lost in a branch-less checkout.

wyldckat

2016-05-25 14:35

updater   ~0006321

I forgot to mention that it might also be necessary to run with the "ParaView-*" folder(s) these commands as well:

  git submodule init
  git submodule update

Because ParaView's git repos also rely on submodules.


As for later removing submodules, the more recent Git versions might make this easier, but the steps presented at the following link seem to be the more reliable ones: http://stackoverflow.com/a/7646931

henry

2016-05-25 14:42

manager   ~0006322

My plan was to simply unpack the ParaView release source pack in ThirdParty-dev, fix it as appropriate and commit it. Then update for each ParaView release. I wasn't planning on tracking the ParaView git repository, that would be too much effort.

henry

2016-05-30 15:46

manager   ~0006333

I cannot find issue #16151 at http://www.paraview.org/Bug do you know if it has been resolved in the ParaView git-repository?

henry

2016-05-30 16:00

manager   ~0006334

After making the changes to the cmake files to allow gcc-6.1 did compilation and installation complete?

I am compling ParaView-5.0.0 and ParaView-5.0.1 with gcc-6.1 and both fail for different reasons but both compile and install correctly with gcc-5.3.1.

wyldckat

2016-05-30 16:09

updater   ~0006335

That report #16151 was taken out recently while Kitware's admin was doing some cleaning up on a massive spam attack they got last week.

This specific issue is already solved in VTK since March 14th: https://github.com/Kitware/VTK/commit/06e2a00bf8accb04db63b3c1c7a454e4afc6fea6 - therefore it's not worth the effort to report this again for ParaView, given that ParaView will sync up with VTK's tree when they see fit to do so.


@Henry: This also gives another +1 in your favour of having a release-based folder of ParaView, instead of forking directly from Kitware's ParaView repo at Github, given that VTK is also a submodule with ParaView's repo...

Nonetheless, having each ParaView-x.x.x folder on a separate repository (then via submodule) does have the advantage of keeping the ThirdParty-dev repository smaller and not requiring to download 60 or more MB for each checkout.

 - Of course the downside is having to later on have to deal with submodule removal, which isn't straight forward at the moment :(

 - On the other hand, after adding a ParaView source code snapshot, the repository will probably only grow 5-15MB per new release, as long as the previous version is not deleted before adding the new version, so that Git can save up space with internal copy-move indexing-tags of file changes.

henry

2016-05-30 16:18

manager   ~0006336

I am having to fiddle with the ParaView-5.0.1 sources quite a bit to get it to compile and install with gcc-6.1 but I hope to get this done today and I will commit the source tree to ThirdParty-dev.

> as long as the previous version is not deleted before adding the new version

Wouldn't it be better to delete the old version before committing the new one? What is the advantage of committing the new one first? They will be in different directories so I don't see how git would work out the correspondence between the files.

wyldckat

2016-05-30 16:29

updater   ~0006337

"git gc" might be able to handle either solution, namely to commit a remove first then commit a new add, or vice versa: https://git-scm.com/book/en/v2/Git-Internals-Packfiles

But I did read in the git manual some years ago about this, but I can't find the page right now... here's a summary on this topic: http://stackoverflow.com/a/7941544 - namely how git detects file copies and renames, based on file content.

henry

2016-05-30 21:26

manager   ~0006339

Resolved in ThirdParty-dev by commit eba760a6d6bb44860fc7d94d922be7215ee93c1f

The patched ParaView-5.0.1 sources are now included in the ThirdParty-dev repository

Issue History

Date Modified Username Field Change
2016-05-24 20:47 projectionist New Issue
2016-05-24 20:47 projectionist File Added: paraView_gcc6_patches.tar.gz
2016-05-24 22:56 wyldckat Project OpenFOAM => ThirdParty
2016-05-24 23:08 wyldckat Project ThirdParty => OpenFOAM
2016-05-24 23:10 wyldckat Category Patch => Bug
2016-05-24 23:11 wyldckat Category Bug => Patch
2016-05-24 23:14 wyldckat Category Patch => Bug
2016-05-24 23:14 wyldckat Project OpenFOAM => ThirdParty
2016-05-24 23:27 wyldckat Note Added: 0006316
2016-05-25 08:41 henry Note Added: 0006317
2016-05-25 12:35 wyldckat Category Bug => Patch
2016-05-25 12:35 wyldckat Product Version dev =>
2016-05-25 12:35 wyldckat Product Version => other
2016-05-25 13:09 wyldckat Note Added: 0006318
2016-05-25 13:32 henry Note Added: 0006319
2016-05-25 14:20 wyldckat Note Added: 0006320
2016-05-25 14:22 wyldckat Note Edited: 0006320
2016-05-25 14:35 wyldckat Note Added: 0006321
2016-05-25 14:42 henry Note Added: 0006322
2016-05-30 15:46 henry Note Added: 0006333
2016-05-30 16:00 henry Note Added: 0006334
2016-05-30 16:09 wyldckat Note Added: 0006335
2016-05-30 16:18 henry Note Added: 0006336
2016-05-30 16:29 wyldckat Note Added: 0006337
2016-05-30 21:26 henry Note Added: 0006339
2016-05-30 21:26 henry Status new => resolved
2016-05-30 21:26 henry Fixed in Version => dev
2016-05-30 21:26 henry Resolution open => fixed
2016-05-30 21:26 henry Assigned To => henry