Skip to content

Conversation

@AryanSalmanpour
Copy link
Member

Motivation

It fixes rocdecDecode sample filesystem issue on SLES

Technical Details

It introduces a check to verify the existence of std::filesystem; if it is unavailable, the code falls back to using std::experimental::filesystem.

JIRA ID

Test Plan

rocDecode CTEST

Test Result

rocDecode CTEST should pass.

Submission Checklist

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a filesystem compatibility issue in the rocdecDecode sample on SLES by adding fallback support for std::experimental::filesystem when std::filesystem is unavailable.

Changes:

  • Adds conditional compilation to detect and use std::filesystem (C++17) or fall back to std::experimental::filesystem
  • Introduces a fs namespace alias to abstract filesystem implementation differences
  • Links against stdc++fs library in CMake to support experimental filesystem
  • Removes color formatting from CMake messages

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
projects/rocdecode/samples/rocdecDecode/rocdecdecode.cpp Adds conditional filesystem header inclusion with namespace aliasing and updates filesystem API calls to use the fs alias
projects/rocdecode/samples/rocdecDecode/CMakeLists.txt Adds stdc++fs library linking and simplifies CMake message formatting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bool b_sort_filenames = false;
if (std::filesystem::is_directory(input_file_path)) {
for (const auto& entry : std::filesystem::directory_iterator(input_file_path)) {
if (fs::is_directory(input_file_path)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this changed from std::filesystem?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rrawther it is changed to support both std::filesystem and std::experimental::filesystem (please see below)

#if __cplusplus >= 201703L && __has_include(<filesystem>)Expand commentComment on line R28Resolved
    #include <filesystem>
    namespace fs = std::filesystem;
#else
    #include <experimental/filesystem>
    namespace fs = std::experimental::filesystem;
#endif

Copy link
Contributor

@rrawther rrawther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants