NatronGitHub / Natron

Open-source video compositing software. Node-graph based. Similar in functionalities to Adobe After Effects and Nuke by The Foundry.
http://NatronGitHub.github.io
GNU General Public License v2.0
4.54k stars 332 forks source link

Fix bugs and add support for UNC paths on Windows. #901

Closed acolwell closed 9 months ago

acolwell commented 1 year ago

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. Additionally, make sure you've done all of these things:

PR Description

What type of PR is this? (Check one of the boxes below)

What does this pull request do?

This pull request fixes a variety of issues with network share paths on Windows in the file dialogs. (#896 )

The change is split into 2 pieces. The first commit mainly just cleans up the code a little bit by introducing some helper functions, removing duplicate code, and introducing unit tests for key FileSystemModel functionality related to paths. No major behavior changes are in this commit. It is mainly just moving stuff around and trying to make the code a little more consistent and readable.

The second commit contains the bug fixes and new logic to support network paths on Windows. It fixes the major issues with navigating network shares and should address all the issues mentioned in (#896).

Have you tested your changes (if applicable)? If so, how?

Yes. I created unit tests to verify the key path manipulation functionality in FileSystemModel. I also built a Windows version of Natron locally and did a bunch of manual tests on the file dialog with network shares. The behavior is MUCH better than it was and the network shares essentially behave just like drives.

devernay commented 10 months ago

Does it fix some or all of the following:

Can another windows developer review this? @rodlie ? This looks OK to me

devernay commented 10 months ago

@acolwell I now have a windows 11 laptop with enough disk space to try things out. Can you confirm that the instructions using MSYS2 from https://github.com/NatronGitHub/Natron/blob/RB-2.5/INSTALL_WINDOWS.md are the way to go? Do you have any plans to do a VS build? Can you please check if the instructions look ok? Or should I prefer this sequence of instructions to install the dependencies:

mkdir ${GITHUB_WORKSPACE}/natron_pacman_repo
cd ${GITHUB_WORKSPACE}/natron_pacman_repo
wget https://github.com/NatronGitHub/Natron/releases/download/windows-mingw-package-repo/natron_package_repo.zip
unzip natron_package_repo.zip
NATRON_REPO_PATH=`cygpath -u $GITHUB_WORKSPACE`
echo -e "#NATRON_REPO_START\n[natron]\nSigLevel = Optional TrustAll\nServer = file://${NATRON_REPO_PATH}/natron_pacman_repo/\n#NATRON_REPO_END" >> /etc/pacman.conf
pacman -Syl natron
pacman -S --needed --noconfirm mingw-w64-x86_64-natron-build-deps-qt5
devernay commented 10 months ago

I installed msys2 using choco install msys2 but uname -s gives MSYS_NT-10.0-22621 which is not handled by the build script. Did I do something wrong? The console has then to be launched using "mingw64", not "msys2"

acolwell commented 10 months ago

Does it fix some or all of the following:

Can another windows developer review this? @rodlie ? This looks OK to me

It fixes #896, but I haven't looked into #236. I'll try to look into that bug in the next day or two. Assuming it goes through the same path abstractions that I fixed, it should work.

acolwell commented 10 months ago

@acolwell I now have a windows 11 laptop with enough disk space to try things out. Can you confirm that the instructions using MSYS2 from https://github.com/NatronGitHub/Natron/blob/RB-2.5/INSTALL_WINDOWS.md are the way to go? Do you have any plans to do a VS build? Can you please check if the instructions look ok? Or should I prefer this sequence of instructions to install the dependencies:

mkdir ${GITHUB_WORKSPACE}/natron_pacman_repo
cd ${GITHUB_WORKSPACE}/natron_pacman_repo
wget https://github.com/NatronGitHub/Natron/releases/download/windows-mingw-package-repo/natron_package_repo.zip
unzip natron_package_repo.zip
NATRON_REPO_PATH=`cygpath -u $GITHUB_WORKSPACE`
echo -e "#NATRON_REPO_START\n[natron]\nSigLevel = Optional TrustAll\nServer = file://${NATRON_REPO_PATH}/natron_pacman_repo/\n#NATRON_REPO_END" >> /etc/pacman.conf
pacman -Syl natron
pacman -S --needed --noconfirm mingw-w64-x86_64-natron-build-deps-qt5

So that page could probably use an update. My "from scratch" workflow roughly matches the windows installer build GitHub action.

  1. Install mingw64 via the installer on https://www.msys2.org/
  2. Open a mingw64 console
  3. pacman -S git mingw-w64-x86_64-wget unzip
  4. Install Natron MINGW pacman repo (https://github.com/NatronGitHub/Natron/releases/tag/windows-mingw-package-repo)
  5. Install Inno Setup (https://jrsoftware.org/isdl.php)
  6. mkdir ~/natron_build
  7. Checkout Natron somewhere (e.g. ~/src/Natron)
  8. cd ~/src/Natron/tools/jenkins
  9. WORKSPACE=~/natron_build BUILD_NAME=natron BUILD_NUMBER=1 BITS=64 NATRON_LICENSE=GPL GIT_URL=https://github.com/NatronGitHub/Natron.git GIT_URL_IS_NATRON=1 SNAPSHOT_BRANCH=RB-2.5 QT_VERSION_MAJOR=5 DISABLE_BREAKPAD=1 NOUPDATE=1 MKJOBS=$(nproc) ./launchBuildMain.sh
  10. Once this finishes an installer zip is located in ~/natron_build/builds_archive/natron/1
  11. Unzip the file in that directory and you'll have a full Natron install.

Once I have this fully built install, my incremental workflow is the following:

  1. pacman -S --needed mingw-w64-x86_64-cmake mingw-w64-x86_64-ninja
  2. Goto Natron source directory (e.g. src/Natron)
  3. mkdir build; cd build
  4. cmake ..
  5. Build: ninja
  6. Copy to Installer dir. : (INSTALLER_ZIP=$(ls ~/natron_build/builds_archive/natron/1/Natron-*Windows-x86_64.zip) ; cp -v App/Natron.exe Tests/Tests.exe $(dirname ${INSTALLER_ZIP})/$(basename ${INSTALLER_ZIP} .zip)/bin/)
  7. Run Natron or Tests in installer dir
  8. Make changes and goto step 5.

Ultimately I'd like to get to a place where I don't need to do a full installer build. Downloading prebuilt test plugin zips like the Natron CI action does is probably the way forward, but I haven't done that yet. I'd also like to get to a place where we aren't using qmake for the installer and cmake for my incrementals. I've just found that the cmake/ninja combo seems faster than the qmake path when doing development. I start with the installer build because that seemed like the "official" way to build releases, but the cmake path seems close enough at this point that I can rely on it for dev purposes.

I have not tried to do a VS build yet especially since we have to build Natron specific versions of certain libraries and at least right now that is easier to do/maintain via pacman packages. Long term, I think it might be possible & easier if we eventual migrated to something like Conan (https://conan.io/center) to build all the things. Over the last few months I have tinkered with that and have almost built all of the plugin repos on all platforms with Conan. At some point I'd like to try getting Natron to build as well, but I just don't have the time/energy to tackle that one yet. For now I've just been focusing on trying to get the existing build system working, repeatable, and a little easier to maintain.

I hope this helps.

acolwell commented 9 months ago

Thanks for the reviews. :)