Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mix of packages built with gcc-8 and gcc-9 lead to crashes when using std::filesystem #639

Closed
j-rivero opened this issue Feb 11, 2022 · 3 comments

Comments

@j-rivero
Copy link
Contributor

j-rivero commented Feb 11, 2022

@nkoenig and other have been reported problems on the destruction of std::filesystem::path gazebosim/gz-sim#1310 (comment). The possible root cause is explained in https://stackoverflow.com/questions/63902528/program-crashes-when-filesystempath-is-destroyed and comes from the fact of building some binary packages with gcc-8 and others with gcc-9 and both software pieces using std::filesystem.

Bionic has gcc-7 so the trick for using gcc-8 should be fine there. Focal is affected. Debian Buster is gcc-8, so I don't expect problems there. As far as I can see the only affected platform is Focal.

Looking into the code the only software using the NEED_C17_COMPILER=true should be transport7 and ignition-gazebo all versions. Some manual checks:

Packages (using std::filesystem) affected:

Packages not affected.

  • ignition-math6 on Focal (built with gcc-9)
  • ignition-utils on Focal (built with gcc-9)
  • sdformat12 stable on Focal. Built with gcc-9 but no std::filesystem.
  • ignition-transport11 on Focal
  • ignition-rendering6 on Focal
  • ignition-sensors6 on Focal
@j-rivero j-rivero changed the title Mix of packages built with gcc-8 and gcc-9 lead to crashes Mix of packages built with gcc-8 and gcc-9 lead to crashes when using std::filesystem Feb 11, 2022
@scpeters
Copy link
Contributor

for whatever reason, ign-gazebo6 is using std::filesystem in https://github.com/ignitionrobotics/ign-gazebo/blob/ign-gazebo6/src/Util.cc#L21

@arjo129
Copy link

arjo129 commented Feb 15, 2022

Seems like we are using it to check if a path is relative: https://github.com/ignitionrobotics/ign-gazebo/blob/faaa3ec2c9006db0be71be318e7f00be51d2a8e3/src/Util.cc#L365 . Would using the check we use for Apple help in this case? Seems like the Apple platform doesn't need std::filesystem.

arjo129 added a commit to gazebosim/gz-common that referenced this issue Feb 16, 2022
This PR adds an `isRelativePath` function to ign-common Filesystem.hh. It is a simple wrapper around `std::filesystem`.  The rationale for this is so we can remove `std::Filesystem` from ign-gazebo as this is causing other issues such as gazebo-tooling/release-tools#639.

Signed-off-by: Arjo Chakravarty <[email protected]>
arjo129 added a commit to gazebosim/gz-sim that referenced this issue Feb 16, 2022
Depends on gazebosim/gz-common#312

See gazebo-tooling/release-tools#639

Basically, this removes the dependency on `std::filesystem` in  ign-gazebo as its been causing issues when binaries are produced by diffrent compiler versions. This current version makes sure that we keep all `std::filesystem` code in` ign-common`.

Signed-off-by: Arjo Chakravarty <[email protected]>
mjcarroll pushed a commit to gazebosim/gz-common that referenced this issue Feb 16, 2022
This PR adds an `isRelativePath` function to ign-common Filesystem.hh. It is a simple wrapper around `std::filesystem`.  The rationale for this is so we can remove `std::Filesystem` from ign-gazebo as this is causing other issues such as gazebo-tooling/release-tools#639.

Signed-off-by: Arjo Chakravarty <[email protected]>
mjcarroll pushed a commit to gazebosim/gz-common that referenced this issue Feb 16, 2022
This PR adds an `isRelativePath` function to ign-common Filesystem.hh. It is a simple wrapper around `std::filesystem`.  The rationale for this is so we can remove `std::Filesystem` from ign-gazebo as this is causing other issues such as gazebo-tooling/release-tools#639.

Signed-off-by: Arjo Chakravarty <[email protected]>
arjo129 added a commit to gazebosim/gz-sim that referenced this issue Mar 2, 2022
Depends on gazebosim/gz-common#312

See gazebo-tooling/release-tools#639

Basically, this removes the dependency on `std::filesystem` in  ign-gazebo as its been causing issues when binaries are produced by diffrent compiler versions. This current version makes sure that we keep all `std::filesystem` code in` ign-common`.

Signed-off-by: Arjo Chakravarty <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>
@j-rivero
Copy link
Contributor Author

Thanks @arjo129 for removing the use of std::filesystem in utils.cc. I think we are done here after latest releases using the right version of compiler. Please reopen if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants