Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ENH: Bundle Qt language tools #6798

Merged
merged 1 commit into from Jan 27, 2023
Merged

ENH: Bundle Qt language tools #6798

merged 1 commit into from Jan 27, 2023

Conversation

lassoan
Copy link
Contributor

@lassoan lassoan commented Jan 26, 2023

Bundle lconvert, lrelease, lupdate tools with the application to allow Slicer modules to update and process translation (.ts) files without requiring Qt installation.

Required for Slicer/SlicerLanguagePacks#1

@lassoan lassoan added this to the Slicer 5.2.2 milestone Jan 26, 2023
@lassoan lassoan requested a review from jcfr January 26, 2023 16:56
@lassoan lassoan self-assigned this Jan 26, 2023
@lassoan
Copy link
Contributor Author

lassoan commented Jan 26, 2023

@jcfr please check if this is a correct way to bundle these Qt utilities. I did not add a separate launcher, as we will use the executables from Slicer's environment.

@pieper it would be great if you could test this on your mac, ideally in the install tree. The best test is to install the latest main version of https://github.com/Slicer/SlicerLanguagePacks and then:

  • select a few languages in Weblate (latest translations) section
  • leave the Qt lrelease path empty in Settings section
  • click Update translation files

jcfr
jcfr reviewed Jan 26, 2023
View reviewed changes
CMake/SlicerCPack.cmake Outdated Show resolved Hide resolved
CMake/SlicerCPack.cmake Outdated Show resolved Hide resolved
CMake/SlicerMacroBuildApplication.cmake Outdated Show resolved Hide resolved
@lassoan @jcfr
ENH: Bundle Qt language tools
b03de71 
Bundle lconvert, lrelease, lupdate tools with the application to allow Slicer modules to update and process translation (.ts) files without requiring Qt installation.

Required for Slicer/SlicerLanguagePacks#1

Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
@lassoan
Copy link
Contributor Author

lassoan commented Jan 26, 2023

Thanks a lot for the comments @jcfr, I've updated the files accordingly.

@jcfr
Copy link
Member

jcfr commented Jan 27, 2023 •
edited

Linux

Testing locally ⌛
Argument displayed in launcher available in build tree image build_tree ✔️ install_tree ✔️
Passing --help argument build_tree ✔️ install_tree ✔️
Update translation files build_tree ✔️ install_tree ✔️

@lassoan
Copy link
Contributor Author

lassoan commented Jan 27, 2023

I've tested on Windows and it worked well in both the build and install trees.

If your linux testing does not bring up any issue then I will merge this before the nightly cutoff time, because the macOS packaging is very slow to do manually.

jcfr
jcfr approved these changes Jan 27, 2023
View reviewed changes
Copy link
Member

@jcfr jcfr left a comment

Choose a reason for hiding this comment

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

Tested after:

  • building the latest version of SlicerLanguagePacks and running from build tree using ./SlicerWithLanguagePacks
  • installing SlicerLanguagePacks extension generate locally and installed into an installed Slicer (based of this PR)

I also reviewed the latest 2 commits of SlicerLanguagePacks, the use of shutil.which should also works nicely on macOS.

@lassoan
Copy link
Contributor Author

lassoan commented Jan 27, 2023

Awesome, thanks a lot for testing. We should include this in Slicer-5.2.2 patch, too.

@lassoan lassoan merged commit 0a00473 into Slicer:main Jan 27, 2023
@lassoan lassoan deleted the bundle-qt-language-tools branch January 27, 2023 02:09
@jcfr jcfr added the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Jan 27, 2023
@jcfr
Copy link
Member

jcfr commented Jan 27, 2023

We should include this in Slicer-5.2.2 patch, too.

Once tonight builds complete and we have a chance to test macOS, I will backport this PR.

@jcfr jcfr mentioned this pull request Jan 27, 2023
Merged
@jcfr jcfr removed the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Feb 1, 2023
@jcfr jcfr added the Domain: i18n Internationalization (language translation) label Mar 15, 2023
@jcfr jcfr mentioned this pull request Mar 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcfr jcfr jcfr approved these changes

Assignees

@lassoan lassoan

Labels
Domain: i18n Internationalization (language translation)
Milestone
Slicer 5.2.2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lassoan @jcfr