cmake: Allow out-of-tree builds #201

jtojnar · 2022-03-10T22:34:10Z

Software on most Linux systems is expected to follow [Filesystem Hierarchy Standard](https://refspecs.linuxfoundation.org/FHS_3.0/fhs/index.html. Let’s update the CMake build to support it so that each Linux distro does not have to do this on its own. On Windows, we can still have it relocatable.

install translations to datadir https://docs.wxwidgets.org/trunk/overview_i18n.html
install and allow loading rest of the stuff from datadir

Fixes: #133

Eventually, we might want to switch to Meson instead of CMake, which might allow us to get rid of the Visual Studio boilerplate since it can generate it.

aardappel

Thanks for working on this!

aardappel · 2022-03-11T00:08:20Z

.gitignore


-CMakeCache.txt


why are these removed from .gitignore ? people may still default to in-tree builds where this is useful

I do not see any value in in-tree builds but re-added that.

aardappel · 2022-03-11T00:11:53Z

Do note that whatever you're changing has to preserve things working as before on Windows / OS X.

Not sure about Meson, I dislike CMake as much as the next guy, but it is a standard many are familiar with.

jtojnar · 2022-03-11T17:45:16Z

Do note that whatever you're changing has to preserve things working as before on Windows / OS X.

As I understand it, CMake is only used on Linux so it should be the same (the C++ code changes should still be compatible). Eventually, it would be nice to use CMake (or Meson) to generate VS and XCode boilerplate, and there we should follow the best practices for each individual platform, not necessarily the current behaviour.

Not sure about Meson, I dislike CMake as much as the next guy, but it is a standard many are familiar with.

I did not realize CMake supports VS backend too, so there is a less of a pressure to switch to Meson.

aardappel · 2022-03-11T17:47:10Z

As I understand it, CMake is only used on Linux so it should be the same.

Yes, but like you said I'd like to keep the door open to eventually migrate over to using CMake everywhere.

aardappel · 2022-03-11T17:51:25Z

CMakeLists.txt

+elseif (WIN32)
+    foreach(locale ${locales})
+        install(
+            FILES "TS/translations/${locale}/ts.mo"


I am not sure what this is going to do. Even if we ever had an "installer creation" on Windows, there's no need to be copying locale files, these files are tiny and the right one is automatically picked up by treesheets anyway. Better to just assume the files will get copied wholesale, and not introduce things which may break in interesting ways in the future.

Same for os x

This will copy the locales to the correct path according to the platform, as described by the wxWidgets docs.

I also want to make msgfmt call part of the build as .mo files are not portable. Possibly, that can cause issues on systems e.g. with different endianness.

That just shows where they would reside relative to the executable on win/osx. In both cases, they'd be in an installer or app package along with all the other standard files, and TreeSheets already knows how to access those, so there is no need for an install action on these platforms.

I am not very familiar with those platforms, but I would expect CMake to generate installers/bundles for those platforms (e.g. https://cmake.org/cmake/help/latest/cpack_gen/nsis.html for Windows). So if we want to use CMake on those platforms, we will also want to specify the platform-specific paths here.

if we ever do so thru CMake, we will do the entire data dir, not just individual locale files. Please remove.

Well, the issue is that the directory tries have different layout from the one used in TS/ on both Windows and Mac. And we also do not want to install junk like the bat files. So we will need to install individual files anyway.

But removed, for now, since I do not have the option to test those platforms.

jtojnar

I have implemented the FHS locale path and tested that the Italian translation works when installed there. There are still some questions to resolve.

jtojnar · 2022-03-11T17:48:28Z

CMakeLists.txt

@@ -24,11 +25,13 @@ cmake_minimum_required(VERSION 3.1)

 project(treesheets)

+include(GNUInstallDirs)
+
+set(TREESHEETS_RELOCATABLE_INSTALLATION OFF CACHE BOOL "Enable to look for data relative to the path treesheets was run from, instead of hardcoding paths from CMAKE_INSTALL_PREFIX")


I wonder if we need this option. The reason I introduced it is that I wanted to prevent relocatable builds from loading the locales from install prefix (e.g. when user has an older version installed on their system and fresh one installed into a testing directory).

I have absolutely no knowledge of install procedures and locales, so can't help with this.

Maybe @probonopd can comment?

Now the option just serves to toggle between the FHS-compatible layout and the layout used on Apple and Windows.

jtojnar · 2022-03-11T17:49:12Z

src/myapp.h

@@ -21,6 +21,9 @@ struct MyApp : wxApp {
        #ifdef __WXGTK__
            locale.AddCatalogLookupPathPrefix(L"/usr");
            locale.AddCatalogLookupPathPrefix(L"/usr/local");


But the FHS paths are already hardcoded (even though they will not work due to missing /locale subdirectory).

.github/workflows/build.yml

jtojnar · 2022-03-11T19:26:19Z

Now it installs all files to their preferred directories on Linux, while (unless I missed something) keeping the old layout on Windows and Mac:

Directory tree

installed
├── bin
│  └── treesheets
└── share
   ├── applications
   │  └── treesheets.desktop
   ├── doc
   │  └── treesheets
   │     ├── docs
   │     │  ├── donations.html
   │     │  ├── file_format_spec.txt
   │     │  ├── history.txt
   │     │  ├── images
   │     │  │  ├── check.png
   │     │  │  ├── documentation
   │     │  │  │  ├── deleted.png
   │     │  │  │  ├── edit.png
   │     │  │  │  ├── grid.png
   │     │  │  │  ├── gridline.png
   │     │  │  │  ├── gridline2.png
   │     │  │  │  ├── gridsel1.png
   │     │  │  │  ├── gridsel2.png
   │     │  │  │  ├── gridsel3.png
   │     │  │  │  ├── insert.png
   │     │  │  │  ├── long1.png
   │     │  │  │  ├── long2.png
   │     │  │  │  ├── long3.png
   │     │  │  │  ├── new.png
   │     │  │  │  ├── postgrid.png
   │     │  │  │  ├── pregrid.png
   │     │  │  │  ├── textsize.png
   │     │  │  │  └── zoomed.png
   │     │  │  ├── excl.png
   │     │  │  ├── screenshots
   │     │  │  │  ├── screenshot_personel.png
   │     │  │  │  ├── screenshot_sales.png
   │     │  │  │  ├── screenshot_todo.png
   │     │  │  │  ├── screenshot_todo_half.png
   │     │  │  │  ├── screenshot_todo_linux.png
   │     │  │  │  ├── screenshot_todo_mac.png
   │     │  │  │  ├── screenshot_tutorial.png
   │     │  │  │  ├── screenshot_tutorial2.png
   │     │  │  │  └── screenshot_unicode.png
   │     │  │  ├── stop.png
   │     │  │  └── treesheets_logo.png
   │     │  ├── screenshots.html
   │     │  ├── script_reference.html
   │     │  └── tutorial.html
   │     ├── examples
   │     │  ├── complex_eval.cts
   │     │  ├── contrib
   │     │  │  ├── help
   │     │  │  │  ├── treesheet key quick reference (by WiM) 3 posted.cts
   │     │  │  │  └── treesheet menu including images landscape final.cts
   │     │  │  ├── Procrastination.cts
   │     │  │  ├── structogram-quicksort.cts
   │     │  │  ├── weekly+calendar.cts
   │     │  │  ├── what-i-like-about-treesheets.cts
   │     │  │  └── yearly-calendar.cts
   │     │  ├── imported_from_xml_examples
   │     │  │  ├── books.cts
   │     │  │  ├── cd_catalog.cts
   │     │  │  ├── menu.cts
   │     │  │  └── plant_catalog.cts
   │     │  ├── operation-reference.cts
   │     │  ├── personel_file.cts
   │     │  ├── sales.cts
   │     │  ├── simple_graph.cts
   │     │  ├── todo_calendar.cts
   │     │  ├── tutorial-de.cts
   │     │  ├── tutorial-fr.cts
   │     │  ├── tutorial.cts
   │     │  ├── unicode_test.cts
   │     │  └── work_breakdown_structure.cts
   │     └── readme.html
   ├── icons
   │  └── hicolor
   │     └── scalable
   │        └── apps
   │           └── treesheets.svg
   ├── locale
   │  ├── de
   │  │  └── LC_MESSAGES
   │  │     └── ts.mo
   │  ├── it
   │  │  └── LC_MESSAGES
   │  │     └── ts.mo
   │  └── zh_CN
   │     └── LC_MESSAGES
   │        └── ts.mo
   └── treesheets
      ├── images
      │  ├── icon16.png
      │  ├── icon32.png
      │  ├── nuvola
      │  │  ├── author
      │  │  ├── dropdown
      │  │  │  ├── apply.png
      │  │  │  ├── back.png
      │  │  │  ├── bookcase.png
      │  │  │  ├── bookmark.png
      │  │  │  ├── bug.png
      │  │  │  ├── cache.png
      │  │  │  ├── cancel.png
      │  │  │  ├── clanbomber.png
      │  │  │  ├── color_line.png
      │  │  │  ├── configure.png
      │  │  │  ├── down.png
      │  │  │  ├── edit_add.png
      │  │  │  ├── edit_remove.png
      │  │  │  ├── email.png
      │  │  │  ├── forward.png
      │  │  │  ├── gohome.png
      │  │  │  ├── history.png
      │  │  │  ├── kalarm.png
      │  │  │  ├── kcmprocessor.png
      │  │  │  ├── kcoloredit.png
      │  │  │  ├── kfm_home.png
      │  │  │  ├── kgpg.png
      │  │  │  ├── kgpg_key3.png
      │  │  │  ├── ksysv.png
      │  │  │  ├── kuser.png
      │  │  │  ├── kweather.png
      │  │  │  ├── ledblue.png
      │  │  │  ├── ledgreen.png
      │  │  │  ├── ledlightblue.png
      │  │  │  ├── ledlightgreen.png
      │  │  │  ├── ledorange.png
      │  │  │  ├── ledpurple.png
      │  │  │  ├── ledred.png
      │  │  │  ├── ledyellow.png
      │  │  │  ├── maybe.png
      │  │  │  ├── messagebox_info.png
      │  │  │  ├── messagebox_warning.png
      │  │  │  ├── misc.png
      │  │  │  ├── mozilla.png
      │  │  │  ├── no.png
      │  │  │  ├── noatunloopsong.png
      │  │  │  ├── package_games_arcade.png
      │  │  │  ├── package_network.png
      │  │  │  ├── package_toys.png
      │  │  │  ├── player_pause.png
      │  │  │  ├── player_play.png
      │  │  │  ├── player_stop.png
      │  │  │  └── up.png
      │  │  ├── fold.png
      │  │  ├── license.txt
      │  │  ├── readme.txt
      │  │  ├── thanks.to
      │  │  └── toolbar
      │  │     ├── editcopy.png
      │  │     ├── editpaste.png
      │  │     ├── filenew.png
      │  │     ├── fileopen.png
      │  │     ├── filesave.png
      │  │     ├── filesaveas.png
      │  │     ├── image.png
      │  │     ├── newgrid.png
      │  │     ├── run.png
      │  │     ├── undo.png
      │  │     ├── zoomin.png
      │  │     └── zoomout.png
      │  ├── render
      │  │  ├── line_ne.png
      │  │  ├── line_nw.png
      │  │  ├── line_se.png
      │  │  └── line_sw.png
      │  ├── treesheets.png
      │  ├── treesheets.svg
      │  └── webalys
      │     ├── readme.txt
      │     └── toolbar
      │        ├── editcopy.png
      │        ├── editpaste.png
      │        ├── filenew.png
      │        ├── fileopen.png
      │        ├── filesave.png
      │        ├── filesaveas.png
      │        ├── image.png
      │        ├── newgrid.png
      │        ├── run.png
      │        ├── undo.png
      │        ├── zoomin.png
      │        └── zoomout.png
      └── scripts
         ├── Export JSON.lobster
         └── modules
            ├── color.lobster
            ├── std.lobster
            ├── stdtype.lobster
            └── vec.lobster

src/myframe.h

aardappel · 2022-03-12T01:26:05Z

ok lgtm, let me know when you think its ready to merge.

jtojnar · 2022-03-12T01:28:26Z

It should be good to merge now (once CI finishes).

jtojnar · 2022-03-12T01:53:14Z

Hmm, looks like we need to bump MACOSX_DEPLOYMENT_TARGET:

error: 'path' is unavailable: introduced in macOS 10.15

This is a best practice, allowing easier development (not having to worry about cleaning the source directory).

We still need to keep MyApp::AddTranslation for cases when the app is built with something other than CMake, and on other platforms than Linux. We are also adding a TREESHEETS_RELOCATABLE_INSTALLATION CMake configuration flag that allows installing to the old relocatable location even on Linux. Finally, we only install the necessary `*.mo` files, not the `*.po` files or `*.bat` helper scripts.

This is needed to make Linux distributions happy. Had to bump minimum macOS version to Catalina since std::filesystem::path is not supported on older versions than 10.15. macOS 10.14 (Mojave) is unsupported since October 2021.

aardappel · 2022-03-12T17:27:43Z

Thanks for your work on this!

- Switch to unstable, using tagged releases is discouraged: aardappel/treesheets#150 (comment) - Also add updateScript to that effect. - CMake is now preferred on Linux, and it installs files to proper locations: aardappel/treesheets#201 - CMake does not set version correctly so we need to do it ourselves. - Format the expression.

jtojnar force-pushed the fhs branch from b645730 to 8d83b5b Compare March 10, 2022 23:51

aardappel reviewed Mar 11, 2022

View reviewed changes

jtojnar mentioned this pull request Mar 11, 2022

treesheets: adjust output directories NixOS/nixpkgs#163492

Closed

13 tasks

jtojnar force-pushed the fhs branch from 8d83b5b to fee178b Compare March 11, 2022 17:38

aardappel reviewed Mar 11, 2022

View reviewed changes

jtojnar commented Mar 11, 2022

View reviewed changes

jtojnar force-pushed the fhs branch 2 times, most recently from a6b0ad2 to 82872e2 Compare March 11, 2022 19:24

jtojnar marked this pull request as ready for review March 11, 2022 19:24

aardappel reviewed Mar 11, 2022

View reviewed changes

src/myframe.h Outdated Show resolved Hide resolved

jtojnar force-pushed the fhs branch 3 times, most recently from 3d9645c to 4b834f0 Compare March 12, 2022 01:22

jtojnar added 4 commits March 12, 2022 02:57

cmake: Allow out-of-tree builds

ead9f6f

This is a best practice, allowing easier development (not having to worry about cleaning the source directory).

cmake: Install desktop file to proper location

7f07921

cmake: Add support for FHS paths for docs and data

68f4282

This is needed to make Linux distributions happy. Had to bump minimum macOS version to Catalina since std::filesystem::path is not supported on older versions than 10.15. macOS 10.14 (Mojave) is unsupported since October 2021.

jtojnar force-pushed the fhs branch from 4b834f0 to 68f4282 Compare March 12, 2022 01:58

aardappel merged commit 4f4ff4b into aardappel:master Mar 12, 2022

jtojnar deleted the fhs branch March 12, 2022 17:28

jtojnar mentioned this pull request Mar 12, 2022

treesheets: 1.0.1 → unstable-2022-03-12 NixOS/nixpkgs#163872

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake: Allow out-of-tree builds #201

cmake: Allow out-of-tree builds #201

jtojnar commented Mar 10, 2022 •

edited

Loading

aardappel left a comment

aardappel Mar 11, 2022

jtojnar Mar 11, 2022

aardappel commented Mar 11, 2022

jtojnar commented Mar 11, 2022

aardappel commented Mar 11, 2022

aardappel Mar 11, 2022

aardappel Mar 11, 2022

jtojnar Mar 11, 2022

aardappel Mar 11, 2022

jtojnar Mar 11, 2022

aardappel Mar 11, 2022

jtojnar Mar 11, 2022

jtojnar left a comment

jtojnar Mar 11, 2022

aardappel Mar 11, 2022

jtojnar Mar 12, 2022

jtojnar Mar 11, 2022

jtojnar commented Mar 11, 2022 •

edited

Loading

aardappel commented Mar 12, 2022

jtojnar commented Mar 12, 2022 •

edited

Loading

jtojnar commented Mar 12, 2022

aardappel commented Mar 12, 2022


		CMakeCache.txt

cmake: Allow out-of-tree builds #201

cmake: Allow out-of-tree builds #201

Conversation

jtojnar commented Mar 10, 2022 • edited Loading

aardappel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aardappel commented Mar 11, 2022

jtojnar commented Mar 11, 2022

aardappel commented Mar 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtojnar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtojnar commented Mar 11, 2022 • edited Loading

aardappel commented Mar 12, 2022

jtojnar commented Mar 12, 2022 • edited Loading

jtojnar commented Mar 12, 2022

aardappel commented Mar 12, 2022

jtojnar commented Mar 10, 2022 •

edited

Loading

jtojnar commented Mar 11, 2022 •

edited

Loading

jtojnar commented Mar 12, 2022 •

edited

Loading