core: add samples::findFile() function #12354
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mshabunin
reviewed
Sep 12, 2018
There was a problem hiding this comment.
IMO, this scheme is too complex. What about hacking into CommandLineParser? It knows argv[0] and can build absolute path to the application being run. Then it can probe two locations:
- relative path
<build>/bin-><src>/samples/data - relative path
<install>/<samples>-><install>/<data>
And an environment variable which will override both.
| @@ -75,6 +75,7 @@ | |||
| @defgroup core_utils_sse SSE utilities | |||
| @defgroup core_utils_neon NEON utilities | |||
| @defgroup core_utils_softfloat Softfloat support | |||
| @defgroup samples_utils Utility functions for OpenCV samples | |||
|
Not sure that this is a reliable way for this:
Adding new string-compatible |
|
Patch is updated:
BTW, |
|
@berak @LaurentBerger @sturkmen72 Any thoughts / suggestions about this patch? |
|
I clone your branch I inserted in a sample : and run it inside vS2017 : Results is OK Now I install opencv using CMakeTargets project and I open windows build/install/samples/cpp and click executable file (or in build/bin/debug) results : Now in cmake-gui I changed cmake install prefix (in f:/tmp/opencv ) results are Now I think for an user who click in cmake and build opencv and may be installed without any change It's Last case (a specific path to install opencv) I think that an user who changes path is not a noobs so may be it's not a problem |
|
@LaurentBerger Thank you for looking on this!
|
|
Inside VS2017 and in window f:/tmp/opencv/samples/cpp I attached my cmakecache.txt : i changed cmake variable CMAKE_INSTALL_PREFIX and all examples are installed in ${CMAKE_INSTALL_PREFIX}/samples PS I copied all dlls in tmp/opencv/samples/cpp ( no conflict my regular install of opencv) |
Actually this breaks the logic. Code assumes that DLLs are located in "bin" directory. And this "bin" directory is added to PATH (to launch ".exe").
Need to enable this for installation of samples data too. Currently there is nothing to search in "install" directory (no files at all). |
|
I have following suggestions:
I think we should cover only two usecases for C++ samples:
Similar for python, but relative locations will be different. All other cases can be considered "Advanced" and will be covered by |
Usecases:
|
|
|
Updated. |
|
Oh, now I see I'm not the first one to try to fix this problem! 😄 Glad to help with the fix for the Python samples in any way. |
|
Some samples are updated. |
|
Any thoughts how to deal with "paths" like these: |
|
@alalek, update: All the methods try to find a single file. As for me, we can leave it for now. |
mshabunin
reviewed
Nov 15, 2018
| if(WIN32) | ||
| file(RELATIVE_PATH INSTALL_DATA_DIR_RELATIVE "${CMAKE_INSTALL_PREFIX}/${OPENCV_BIN_INSTALL_PATH}" "${CMAKE_INSTALL_PREFIX}/${OPENCV_OTHER_INSTALL_PATH}") | ||
| else() | ||
| file(RELATIVE_PATH INSTALL_DATA_DIR_RELATIVE "${CMAKE_INSTALL_PREFIX}/${OPENCV_LIB_INSTALL_PATH}" "${CMAKE_INSTALL_PREFIX}/${OPENCV_OTHER_INSTALL_PATH}") |
There was a problem hiding this comment.
Here OPENCV_SAMPLES_SRC_INSTALL_PATH is used to determine relative location, but in samples/CMakeLists.txt, data directory is installed to the OPENCV_OTHER_INSTALL_PATH location.
mshabunin
approved these changes
Nov 15, 2018
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TODO:
cmake ... "-DOPENCV_INSTALL_DATA_DIR_RELATIVE=../../../../sources"