refactor: Replace fs::unique_path with GetUniquePath(path) calls #21052

1. 
   kiminuo commented at 12:47 pm on February 1, 2021: contributor

   This PR makes it easier in #20744 to remove our dependency on the boost::filesystem::unique_path() function which does not have a direct equivalent in C++17.

   This PR attempts to re-implement boost::filesystem::unique_path() as GetUniquePath(path) but the implementations are not meant to be the same.

   Note:

   • Boost 1.75.0 implementation of unique_path: https://github.com/boostorg/filesystem/blob/9cab675b71e98706886a87afe7c19eb9da568961/src/unique_path.cpp#L235

   • In the previous implementation, I attempted to add:

     0fs::path GetUniquePath(const fs::path& base)
     1{
     2    FastRandomContext rnd;
     3    fs::path tmpFile = base / HexStr(rnd.randbytes(8));
     4    return tmpFile;
     5}
     

     to fs.cpp but this leads to a circular dependency: “fs -> random -> logging -> fs”. That is why the modified implementation adds a new file.

2. DrahtBot added the label Utils/log/libs on Feb 1, 2021
3. 
   DrahtBot commented at 1:27 pm on February 1, 2021: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #20744 ([POC] Use std::filesystem. Remove Boost Filesystem & System by fanquake)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. fanquake requested review from ryanofsky on Feb 2, 2021
5. fanquake renamed this:
   [draft] refactor: Replace fs::unique_path with GetUniquePath(path) calls
   refactor: Replace fs::unique_path with GetUniquePath(path) calls
   on Feb 2, 2021
6. 
   laanwj commented at 12:04 pm on February 2, 2021: member

   Concept ACK

   However, this introduces a circular dependency right now:

   0A new circular dependency in the form of "fs -> random -> logging -> fs" appears to have been introduced.
   

7. kiminuo force-pushed on Feb 2, 2021
8. in src/test/fs_tests.cpp:84 in 805f8884f0 outdated

   79+        BOOST_CHECK_EQUAL(tmpfolder, p1.parent_path());
   80+        BOOST_CHECK_EQUAL(tmpfolder, p2.parent_path());
   81+        BOOST_CHECK_EQUAL(tmpfolder, p3.parent_path());
   82+
   83+        // Ensure that generated paths are actually different.
   84+        BOOST_CHECK(p1 != p2);
   

   ---

   ---

   

   kiminuo commented at 10:17 am on February 3, 2021:

   I have left this intentionally very simple. One can certainly come up with many different ways to make this more robust but I would like to solicit for feedback for this.
9. 
   kiminuo commented at 10:17 am on February 3, 2021: contributor

   However, this introduces a circular dependency right now:

   I have changed the implementation. So this is fixed.

10. kiminuo marked this as ready for review on Feb 3, 2021
11. DrahtBot added the label Needs rebase on Feb 4, 2021
12. Introduce GetUniquePath(base) helper method to replace boost::filesystem::unique_path() which is not available in std::filesystem. 1bca2aa694
13. kiminuo force-pushed on Feb 4, 2021
14. DrahtBot removed the label Needs rebase on Feb 4, 2021
15. 
    practicalswift commented at 8:54 am on February 5, 2021: contributor

    Concept ACK

    Thanks for paving the way for further Boost removal!

16. in src/test/fs_tests.cpp:90 in 1bca2aa694

    86+        BOOST_CHECK(p1 != p3);
    87+    }
    88 }
    89 
    90-BOOST_AUTO_TEST_SUITE_END()
    91+BOOST_AUTO_TEST_SUITE_END()
    

    ---

    ---

    

    ryanofsky commented at 4:17 pm on February 5, 2021:

    In commit “Introduce GetUniquePath(base) helper method to replace boost::filesystem::unique_path() which is not available in std::filesystem.” (1bca2aa694cd85984c09699ae28daec313077462)

    Not a problem but there is an extraneous whitespace change here

17. ryanofsky approved
18. 
    ryanofsky commented at 4:24 pm on February 5, 2021: member

    Code review ACK 1bca2aa694cd85984c09699ae28daec313077462. It’s a simple change and extra test coverage is nice
19. 
    laanwj commented at 9:19 pm on February 9, 2021: member

    Code review ACK 1bca2aa694cd85984c09699ae28daec313077462
20. laanwj merged this on Feb 9, 2021
21. laanwj closed this on Feb 9, 2021

    ---

22. kiminuo deleted the branch on Feb 9, 2021
23. sidhujag referenced this in commit 92f9cd5b8f on Feb 11, 2021
24. kittywhiskers referenced this in commit cec4f1fe48 on Jul 2, 2021
25. kittywhiskers referenced this in commit 8a06d24938 on Jul 4, 2021
26. kittywhiskers referenced this in commit 08178e5e82 on Jul 9, 2021
27. kittywhiskers referenced this in commit f1ad65cf66 on Jul 9, 2021
28. kittywhiskers referenced this in commit cecac0acf2 on Jul 9, 2021
29. kittywhiskers referenced this in commit aacf67cdcd on Jul 13, 2021
30. kittywhiskers referenced this in commit 408c9e8ce3 on Jul 13, 2021
31. kittywhiskers referenced this in commit 1767ac59e3 on Jul 13, 2021
32. kittywhiskers referenced this in commit da1061751d on Jul 15, 2021
33. kittywhiskers referenced this in commit 9657ce4219 on Jul 15, 2021
34. kittywhiskers referenced this in commit e63a2cbb3d on Jul 16, 2021
35. kittywhiskers referenced this in commit 81112fd9d8 on Aug 1, 2021
36. kittywhiskers referenced this in commit d6738e4cdc on Aug 5, 2021
37. kittywhiskers referenced this in commit 874489e263 on Aug 5, 2021
38. kittywhiskers referenced this in commit 5e0cc08663 on Aug 5, 2021
39. kittywhiskers referenced this in commit 09ace18f03 on Aug 9, 2021
40. UdjinM6 referenced this in commit 7aebf156e9 on Aug 10, 2021
41. 5tefan referenced this in commit 1313be7d97 on Aug 12, 2021
42. fanquake referenced this in commit 69a439b880 on Sep 8, 2021
43. MarcoFalke referenced this in commit 5e3380b9f5 on Sep 8, 2021
44. sidhujag referenced this in commit 8aec98251d on Sep 11, 2021
45. PastaPastaPasta referenced this in commit e70bdb8fe6 on Mar 13, 2022
46. gades referenced this in commit 2f7541ed5e on May 1, 2022
47. gades referenced this in commit f50228210a on May 1, 2022
48. DrahtBot locked this on Aug 18, 2022

Contributors
kiminuo DrahtBot laanwj practicalswift ryanofsky

Labels
Utils/log/libs