CGAL Data: doc + more data moves#6034
Conversation
| /// pointing to the data directory of the \cgal version used. | ||
| /// The function will make an attempt to open the file at the location returned | ||
| /// and will print an error message on `std::cerr` if the file could not be opened. | ||
| std::string data_file_path(const std::string& filename); |
There was a problem hiding this comment.
I would that to be tested, in various configurations. In particular, I wonder what happens if CGAL is installed. Are data installed as well, and where?
There was a problem hiding this comment.
I would that to be tested, in various configurations. In particular, I wonder what happens if CGAL is installed. Are data installed as well, and where?
It can be tested in Installation/test/Installation/test_configuration.cpp: that file is compiled and run in a lot of various configuration/installation of CGAL, both in the CI (for the Git layout), and in the testsuite (for releases layout).
There was a problem hiding this comment.
Simply add a thumb up if you don't have any comment so that we know that it has been reviewed.
See also the text added in the developer wiki:
https://github.com/CGAL/cgal/wiki/Directory-Structure-for-Packages#shared-data-directory
There was a problem hiding this comment.
Simply add a thumb up if you don't have any comment so that we know that it has been reviewed.
See also the text added in the developer wiki: CGAL/cgal/wiki/Directory-Structure-for-Packages#shared-data-directory
My review of the wiki section led to the fix of a typo: https://github.com/CGAL/cgal/wiki/Directory-Structure-for-Packages/_compare/4545a717e0e2802299b894ec3a6c9b96adf0ace3...8fb212d5989c65b1274386f1096232cda485323e
There was a problem hiding this comment.
@lrineau I agree but it does not have anything to do with the review of the doc (or maybe you want to know what I meant by standard). Examples are not installed so neither will data be. About the -example packages of linux distributions, I haven't done anything but with your feedback and @joachim-reichel 's I can update scripts accordingly.
There was a problem hiding this comment.
Let me sketch my idea for .deb packaging: Since data files are only used by demos/examples (tests are not included in the tarball), I would probably not ship them with the library/header packages, but in the -example package (actually libcgal-demo). This package contains two tarballs for the demos/examples, and I would probably add another one for the data files. The user can extract them anywhere on the system (probably in the home directory). Will the data directory be automatically found, if "data" is e.g. next to "examples" (as in a tarball layout)? (There will be only the "examples"/"demo" and "data" directory trees, everything else is either installed or not present at all.)
There was a problem hiding this comment.
Right now, the cases of automatic detection of data dir are only done with reference to the CGAL_DIR (be it a branch checkout or a regular release). I guess we can probably add another fallback rule based on CMAKE_SOURCE_DIR. Thinking about it, I'm in favor of adding another rule only if the data dir is in the same archive as the examples so that the layout is somehow imposed. What do you think?
There was a problem hiding this comment.
I think the data files should be in the same tarball as the examples, and in the same component of the CMake installation rules.
And the automatic detection should work:
- when CGAL is installed, with or without examples,
- or when a user decompress the examples tarball in the home directory.
Co-authored-by: Mael <mael.rouxel.labbe@geometryfactory.com>
|
Successfully tested in CGAL-5.4-Ic-82 (extra step can be done in another PR to avoid issue with branches using images) |
lrineau
added
the
rm only: ready for master
lrineau
removed
the
rm only: ready for master
4 participants
Follow up of #4229 and #5427
TODO: