Skip to content

Fix/cmake run dt from build dir#5086

Merged
TurboGit merged 88 commits into
darktable-org:masterfrom
codingdave:fix/cmake-run-dt-from-build-dir
Oct 15, 2020
Merged

Fix/cmake run dt from build dir#5086
TurboGit merged 88 commits into
darktable-org:masterfrom
codingdave:fix/cmake-run-dt-from-build-dir

Conversation

@codingdave
Copy link
Copy Markdown
Contributor

@codingdave codingdave commented May 16, 2020

This is my proposal for being able to run darktable without installing it, meaning out of binary tree runs.

Basically it is expected from CMake projects to have a working program in the build directory. Until now the user had to build and install darktable. This PR eliminates the install requirement because the application runs from the binary tree, wherever that is. This makes dt more compatible to CMake capable IDEs like e.g. Visual Studio Code.

There is just a minimum change to the CMake structure. Basically the idea is instead of using absolute pathes to generate relative pathes from CMake that can be resolved at runtime from both, the installed location and the build-tree. The relative path then is expanded at runtime in C.

I was removing redundancies and created functions for the common part.

FIXED: One change that I am not so sure about happened: Before my PR the function dt_loc_init_tmp_dir returns 1 in case of an error, whereas all the other dt_loc_init_xxx_dir functions were exiting with exit(EXIT_FAILURE);. Since I put all that code in one function no 1 is returned any longer for the temporary directory, but a exit is triggered like for any other directory. I think consistency wins but I might have overlooked something here.

FIXED: Also whats strange for me is that I get A component of file_name does not name an existing file or file_name points to an empty string: messages for existing files/directories. I am not sure what causes this (realpath returns ENOENT).

I'm happy about feedback.
Kind regards


Possible startup calls:

  • ./darktable
  • ./some/path/to/darktable
  • ~/some/path/to/darktable
  • ~$(username)/some/path/to/darktable
  • darktable (from anywhere on the file system while starting darktable from PATH)
  • OsX: having the bundle installed and clicking the dt icon
  • OsX: having the bundle open (mounted) and starting from the mount position

codingdave added 26 commits May 16, 2020 17:14
A component of file_name does not name an existing file or file_name points to an empty string: /home/david/.config/darktable/library.db
@codingdave codingdave force-pushed the fix/cmake-run-dt-from-build-dir branch from e344c47 to cb5ad8b Compare May 16, 2020 15:15
@codingdave
Copy link
Copy Markdown
Contributor Author

codingdave commented May 16, 2020

I have no idea why this fails on travis. I cannot reproduce failing tests locally running ci-script.sh. Any suggestions?

@dterrahe
Copy link
Copy Markdown
Contributor

This definitely looks like something I want. At the moment, on debian unstable, I get:

A component of file_name does not name an existing file or file_name points to an empty string: /opt/darktable.test/bin/../share/darktable
A component of file_name does not name an existing file or file_name points to an empty string: /opt/darktable.test/bin/../lib/darktable
A component of file_name does not name an existing file or file_name points to an empty string: /opt/darktable.test/bin/../share/darktable/locale
directory for darktable.localedir has not been set.

Also, it looks like a lot was installed in the install target directory while I build the "all" target. Maybe I'm confused.

@codingdave
Copy link
Copy Markdown
Contributor Author

@dterrahe
Do you get the message A component of file_name does not name an existing file or file_name points to an empty string for non-existing files or directories only? I get this also for my existing database. No idea why.
What seems odd is that darktable.localedir wasnt set. Can you please check with -d dev, you should see all currently set/found directories. Please be aware that you need to have the CWD set to the one from your darktable executable.

Also, it looks like a lot was installed in the install target directory while I build the "all" target. Maybe I'm confused.
This should not have happened due to my PR. Can you please check if the vanilla dt behaves the same? If not, can you please provide the tree of all files that are installed already?

Copy link
Copy Markdown
Contributor Author

@codingdave codingdave left a comment

Choose a reason for hiding this comment

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

all requested changes have been made - afaik

Copy link
Copy Markdown
Member

@parafin parafin left a comment

Choose a reason for hiding this comment

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

I couple more minor comments, but otherwise this PR works for me.

Comment thread CMakeLists.txt
Comment thread src/libs/CMakeLists.txt Outdated
Comment thread src/osx/osx.mm Outdated
Comment thread CMakeLists.txt Outdated
Comment thread src/common/file_location.c
Copy link
Copy Markdown
Member

@parafin parafin left a comment

Choose a reason for hiding this comment

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

Clarification needed on bindir handling.

Comment thread CMakeLists.txt
Comment thread CMakeLists.txt Outdated
@parafin
Copy link
Copy Markdown
Member

parafin commented Oct 12, 2020

Github shows that some changes are still requested, but I don't see any unresolved comments. I'm not sure how it works or why it happened.

Anyway, I think the only thing left is confirmation that current version of PR works on Windows too.

@codingdave
Copy link
Copy Markdown
Contributor Author

right @parafin, I also don't understand that. Im installing the windows system right now to verify.

@codingdave
Copy link
Copy Markdown
Contributor Author

codingdave commented Oct 14, 2020

@parafin took some time but finally got it compiled and it runs relatively, as planned. Can you confirm that it works on OSX?

$ ~/darktable/build/bin/darktable.exe -d dev

0.000000 application_directory: C:\msys64\home\david\darktable\build\bin
0.000000 darktable.datadir: C:\msys64\home\david\darktable\build\share\darktable
0.000000 darktable.plugindir: C:\msys64\home\david\darktable\build\lib\darktable
0.000000 darktable.localedir: C:\msys64\home\david\darktable\build\share\locale
0.013990 darktable.configdir: C:\Users\david\AppData\Local\darktable
0.016988 darktable.cachedir: C:\Users\david\AppData\Local\Microsoft\Windows\INetCache\darktable
0.016988 darktable.sharedir: C:\msys64\home\david\darktable\build\share\darktable
0.016988 darktable.tmpdir: C:\msys64\tmp
0.016988 new_xdg_data_dirs: (null)
0.708892 kernel directory: C:\msys64\home\david\darktable\build\share\darktable\kernels
[l10n] error: can't open iso-codes file C:\msys64\home\david\darktable\build\share\darktable\..\iso-codes\json\iso_639-2.json' there won't be nicely translated language names in the preferences. [dt_pthread_create] info: bumping pthread's stacksize from 0 to 2097152 ... [iop_lens]: could not load lensfun database in C:\msys64\home\david\darktable\build\share\lensfun\version_2'!
(darktable.exe:26116): GLib-CRITICAL **: 22:09:01.324: g_str_has_prefix: assertion 'str != NULL' failed
...

Regarding the l10n error, Im checking now...

@TurboGit this PR took so much time and effort I will not rebase this into smaller chunks. So feel free to merge it as you like when this is done. Thanks in advance.

@TurboGit
Copy link
Copy Markdown
Member

So this is ready for merging! Great work ! I'll test.

@TurboGit
Copy link
Copy Markdown
Member

I cannot build:


<ROOT_DIR>/darktable/src/external/whereami/src/whereami.c

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm
  .hpp .hxx .in .txx


CMake Error at src/external/CMakeLists.txt:5 (add_library):
  No SOURCES given to target: whereami

And indeed into the directory whereami I have no file:

$ ls -l ./src/external/whereami/
total 0

Are you missing a file in a commit ?

@TurboGit
Copy link
Copy Markdown
Member

A note I have of course deleted build directory:

$ rm -fr build ; ./build.sh

@codingdave
Copy link
Copy Markdown
Contributor Author

codingdave commented Oct 14, 2020 via email

@TurboGit
Copy link
Copy Markdown
Member

A new submodule indeed :) The build went fine. What I'll do is squash everything together so we will have a single commit that could be reverted locally to build with MSYS in case of troubles. I'll merge today to ensure more field testing as I know many people are building on Windows. Thanks for the hard work!

@codingdave
Copy link
Copy Markdown
Contributor Author

Thank you @TurboGit. I have trouble building on windows and such testing the PR. Also I did not change how l10n is resolved on windows. This hardcoded approach should not get broken by my changeset. I agree, more field testing would be helpful here. Also updating the windows build instructions. I can file some suggestions as PR but I might not be the best person to decide what is good and what is bad in the Windows MSYS2 environment with the interactions to MINGW64, MINGW32, CYGWIN, and what else.

@TurboGit TurboGit merged commit 3a42bcc into darktable-org:master Oct 15, 2020
@codingdave codingdave deleted the fix/cmake-run-dt-from-build-dir branch October 18, 2020 15:18
AlynxZhou added a commit to AlynxZhou/ansel that referenced this pull request Dec 7, 2023
In order to support run-from-build-dir[0], what basically did is
"manually do what will be done during installation in the build time",
that contains a mixture of change binary_dir of "change binary_dir of
subdirectories to pretend it as the same structure of install dir" and
"manually copy files to build dir to pretend it as the same structure of
install dir". This changes the structure of build dir, adds a lot of
variables into CMake files, and often use variables wrongly.

Because this makes CMake files hard to read (the variables are
misleading), and running desktop apps directly from build dir is not a
recommended way to user, this commit will drop it.

User should always install it before running it, it is easy to install
to where a normal user has permission by changing
`CMAKE_INSTALL_PREFIX`.

Loading resources and plugins relatively is kept so the app is still
relocatable.

[0]: darktable-org#5086
AlynxZhou added a commit to AlynxZhou/ansel that referenced this pull request Dec 7, 2023
In order to support run-from-build-dir[0], what basically did is
"manually do what will be done during installation in the build time",
that contains a mixture of change binary_dir of "change binary_dir of
subdirectories to pretend it as the same structure of install dir" and
"manually copy files to build dir to pretend it as the same structure of
install dir". This changes the structure of build dir, adds a lot of
variables into CMake files, and often use variables wrongly.

Because this makes CMake files hard to read (the variables are
misleading), and running desktop apps directly from build dir is not a
recommended way to user, this commit will drop it.

User should always install it before running it, it is easy to install
to where a normal user has permission by changing
`CMAKE_INSTALL_PREFIX`.

Loading resources and plugins relatively is kept so the app is still
relocatable.

[0]: darktable-org#5086
AlynxZhou added a commit to AlynxZhou/ansel that referenced this pull request Dec 8, 2023
In order to support run-from-build-dir[0], what basically did is
"manually do what will be done during installation in the build time",
that contains a mixture of change binary_dir of "change binary_dir of
subdirectories to pretend it as the same structure of install dir" and
"manually copy files to build dir to pretend it as the same structure of
install dir". This changes the structure of build dir, adds a lot of
variables into CMake files, and often use variables wrongly.

Because this makes CMake files hard to read (the variables are
misleading), and running desktop apps directly from build dir is not a
recommended way to user, this commit will drop it.

User should always install it before running it, it is easy to install
to where a normal user has permission by changing
`CMAKE_INSTALL_PREFIX`.

Loading resources and plugins relatively is kept so the app is still
relocatable.

[0]: darktable-org#5086
aurelienpierre pushed a commit to AlynxZhou/ansel that referenced this pull request Dec 12, 2023
In order to support run-from-build-dir[0], what basically did is
"manually do what will be done during installation in the build time",
that contains a mixture of change binary_dir of "change binary_dir of
subdirectories to pretend it as the same structure of install dir" and
"manually copy files to build dir to pretend it as the same structure of
install dir". This changes the structure of build dir, adds a lot of
variables into CMake files, and often use variables wrongly.

Because this makes CMake files hard to read (the variables are
misleading), and running desktop apps directly from build dir is not a
recommended way to user, this commit will drop it.

User should always install it before running it, it is easy to install
to where a normal user has permission by changing
`CMAKE_INSTALL_PREFIX`.

Loading resources and plugins relatively is kept so the app is still
relocatable.

[0]: darktable-org#5086
aurelienpierre pushed a commit to aurelienpierreeng/ansel that referenced this pull request Dec 12, 2023
In order to support run-from-build-dir[0], what basically did is
"manually do what will be done during installation in the build time",
that contains a mixture of change binary_dir of "change binary_dir of
subdirectories to pretend it as the same structure of install dir" and
"manually copy files to build dir to pretend it as the same structure of
install dir". This changes the structure of build dir, adds a lot of
variables into CMake files, and often use variables wrongly.

Because this makes CMake files hard to read (the variables are
misleading), and running desktop apps directly from build dir is not a
recommended way to user, this commit will drop it.

User should always install it before running it, it is easy to install
to where a normal user has permission by changing
`CMAKE_INSTALL_PREFIX`.

Loading resources and plugins relatively is kept so the app is still
relocatable.

[0]: darktable-org#5086
lologor pushed a commit to lologor/ansel that referenced this pull request Dec 12, 2023
In order to support run-from-build-dir[0], what basically did is
"manually do what will be done during installation in the build time",
that contains a mixture of change binary_dir of "change binary_dir of
subdirectories to pretend it as the same structure of install dir" and
"manually copy files to build dir to pretend it as the same structure of
install dir". This changes the structure of build dir, adds a lot of
variables into CMake files, and often use variables wrongly.

Because this makes CMake files hard to read (the variables are
misleading), and running desktop apps directly from build dir is not a
recommended way to user, this commit will drop it.

User should always install it before running it, it is easy to install
to where a normal user has permission by changing
`CMAKE_INSTALL_PREFIX`.

Loading resources and plugins relatively is kept so the app is still
relocatable.

[0]: darktable-org#5086
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

difficulty: average some changes across different parts of the code base feature: enhancement current features to improve scope: codebase making darktable source code easier to manage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants