CDrummond / cantata

Qt5 Graphical MPD Client
GNU General Public License v3.0
1.04k stars 184 forks source link

MacOS fixes #1711

Closed dphoyes closed 3 years ago

CDrummond commented 3 years ago

This seems to be mainly reverting https://github.com/CDrummond/cantata/pull/1703 - can you discuss with the author of that pull request as to which is the correct method? I don't have access to macOS so am nt willing to keep switching back and forth.

dphoyes commented 3 years ago

I just saw that #1709 exists. It looks like the same issue, so it's likely this PR fixes it.

Marcool04 commented 3 years ago

Hum… ok, we obviously have some kind of issue with case sensitivity. Indeed, as the master branch stands, I also get the following error on make install:

Install the project...
-- Install configuration: "Release"
-- Installing: /Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/Cantata.app/Contents/Resources/scripts/cantata-dynamic
CMake Error at cmake_install.cmake:61 (file):
  file INSTALL cannot find
  "/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/Cantata.app/Contents/MacOS/cantata":
  No such file or directory.

But when I apply your commit https://github.com/CDrummond/cantata/pull/1711/commits/96795d6a9156416d2d5ab70c60cd9c165a901734 reverting most of my changes from https://github.com/CDrummond/cantata/pull/1703/commits/32d9d6b387f1e0f4f895cb2db0551fd28ca959b5 I get a different error, which actually causes make install to fail earlier:

-- fixup_bundle
--   app='/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app'
--   libs='/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app/Contents/PlugIns/imageformats/libqjpeg.dylib;/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app/Contents/PlugIns/imageformats/libqsvg.dylib;/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app/Contents/PlugIns/iconengines/libqsvgicon.dylib;/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app/Contents/PlugIns/platforms/libqcocoa.dylib;/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app/Contents/PlugIns/styles/libqmacstyle.dylib;/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app/Contents/PlugIns/sqldrivers/libqsqlite.dylib;/Users/mark/Software/Contributions/Cantata/CDrummond-cantata/install/cantata.app/Contents/PlugIns/mediaservice/libqavfmediaplayer.dylib'
--   dirs=''
--   ignoreItems=''
-- warning: *NOT* handled - .app directory case...
CMake Error at /usr/local/Cellar/cmake/3.20.2/share/cmake/Modules/BundleUtilities.cmake:997 (message):
  error: fixup_bundle: not a valid bundle
Call Stack (most recent call first):
  /Users/mark/Software/Contributions/Cantata/CDrummond-cantata/src/cmake/DeployQt5.cmake:193 (fixup_bundle)
  cmake_install.cmake:163 (FIXUP_QT5_EXECUTABLE)

[SNIP]
make: *** [install] Error 1

I am running macOS Catalina, and this is what my main system drive shows:

$ diskutil info /dev/disk1s5
   Device Identifier:         disk1s5
   Device Node:               /dev/disk1s5
   Whole:                     No
   Part of Whole:             disk1

   Volume Name:               Mac new
   Mounted:                   Yes
   Mount Point:               /

   Partition Type:            41504653-0000-11AA-AA11-00306543ECAC
   File System Personality:   Case-sensitive APFS

[SNIP]
$ diskutil info /dev/disk1s1
   Device Identifier:         disk1s1
   Device Node:               /dev/disk1s1
   Whole:                     No
   Part of Whole:             disk1

   Volume Name:               Mac new - Data
   Mounted:                   Yes
   Mount Point:               /System/Volumes/Data

   Partition Type:            41504653-0000-11AA-AA11-00306543ECAC
   File System Personality:   Case-sensitive APFS

Are you, by any chance, running on a case-insensitive drive?

I'll keep experimenting, maybe I can figure out some middle-ground that works on both… Sorry for the mess. Mark.

dphoyes commented 3 years ago

Ah, I do indeed have a case-insensitive drive. I'll un-revert the case-related changes.

CDrummond commented 3 years ago

If this merge request now fixes the build for both of you I will merge. Please confirm. (And thanks).

Marcool04 commented 3 years ago

Sorry @CDrummond for all this trouble… I'm testing now, will let you know as soon as build is over.

CDrummond commented 3 years ago

@Marcool04 Its no trouble. Just that I have no macOS to test against, so I have no idea who is correct.

Marcool04 commented 3 years ago

Well I feel we're pretty lucky to have two different systems—@dphoyes' and mine—that differ in this strange but key way (case-{in,}sensitive fs), to test against. Hopefully working together we'll get something worked out 👍

Marcool04 commented 3 years ago

Sorry @dphoyes, I just cloned your fork and still get this error at the end of make install:

Install the project...
-- Install configuration: "Release"
-- Installing: /Users/mark/Software/Contributions/Cantata/dphoyes-cantata-git/install/Cantata.app/Contents/Resources/scripts/cantata-dynamic
CMake Error at cmake_install.cmake:61 (file):
  file INSTALL cannot find
  "/Users/mark/Software/Contributions/Cantata/dphoyes-cantata-git/build/Cantata.app/Contents/MacOS/cantata":
  No such file or directory.

make: *** [install] Error 1

I'll take a look in the morning and let you all know when I've figure anything out.

dphoyes commented 3 years ago

@Marcool04 Can you give the new version a try? It works for me, but I don't have a case-sensitive filesystem to test on.

Marcool04 commented 3 years ago

Hey that works! 👍

I was just trying to figure out why there was a cantata.app still in the build directory when finished. I had a rather long patch ready that actually skips the step that you modified, where the cantata binary inside Contents/MacOS is moved back up into the install directory. But what you're proposing seems to work fine, and there's no reason not to have that intermediary cantata.app in the build directory. So, @CDrummond, this pull request LGTM, if that's sufficient validation to you which course to follow…