duti: update to new darwin SDK pattern #357745
Conversation
|
Result of 1 package built:
|
ofborg
bot
added
6.topic: darwin
n-hass
force-pushed
the
update-duti-applesdk
branch
from
75103ec
to
2d898d5
Compare
| configureFlags = [ | ||
| "--with-macosx-sdk=/homeless-shelter" | ||
| nativeBuildInputs = [ autoreconfHook ]; | ||
| buildInputs = lib.optional stdenv.hostPlatform.isDarwin apple-sdk_11; |
There was a problem hiding this comment.
Are you sure this is necessary? It does not look like it uses any macOS 11 APIs, so buildInputs should be able to be omitted entirely.
| # needed to prevent duti from trying to guess our sdk | ||
| # NOTE: this is different than stdenv.hostPlatform.config! | ||
| configureFlags = | ||
| lib.optionals stdenv.hostPlatform.isAarch64 [ "--host=aarch64-apple-darwin18" ] | ||
| ++ lib.optionals stdenv.hostPlatform.isx86_64 [ "--host=x86_64-apple-darwin18" ]; |
There was a problem hiding this comment.
Looking at https://github.com/moretension/duti/blob/8b5b9a08702d5de6a5bce57fade593fca46e126e/aclocal.m4, we do actually still need to work around their outdated SDK checks, but we should do it a little differently.
I suggest:
preConfigure = ''
configureFlagsArray+=(
"--with-macosx-sdk=$SDKROOT"
"--with-macosx-deployment-target=$MACOSX_DEPLOYMENT_TARGET"
)
'';This should also obviate the need for --host.
There was a problem hiding this comment.
Sure, changed. But I can’t see how to remove the host flag - you still end up with failure on newer systems as host will be set to an unhandled version (eg darwin24 on latest macOS). Any suggestions, or are you happy with the latest commit?
There was a problem hiding this comment.
Hmm, that’s very unfortunate, it’s doing some strange and unnecessary stuff. I see two options:
-
Pass
--host=${stdenv.hostPlatform.parsed.cpu.name}-apple-darwin20and putsubstituteInPlace Makefile.in --replace-fail '@macosx_arches@' ""in apostPatchphase to work around its suboptimal architecture selection (might be able to get away without the latter part, not sure). -
Add a patch that comments out
DUTI_CHECK_SDKandDUTI_CHECK_DEPLOYMENT_TARGETinconfigure.acand replaces all this with justOPTOPTS= @OPTOPTS@. Then we don’t need any configure flags at all and it should just work indefinitely. This is probably the cleaner approach.
There was a problem hiding this comment.
Great, thanks. I’ve added the patch
ofborg
bot
removed
the
2.status: merge conflict
ofborg
bot
added
11.by: package-maintainer
n-hass
force-pushed
the
update-duti-applesdk
branch
2 times, most recently
from
07a5f39
to
928f763
Compare
remove old SDK, patch build configure checks Signed-off-by: Nicholas Hassan <[email protected]>
n-hass
force-pushed
the
update-duti-applesdk
branch
from
928f763
to
ed8e726
Compare
There was a problem hiding this comment.
Looks good, thanks!
nixpkgs-review result
Generated using nixpkgs-review.
Command: nixpkgs-review pr 357745
aarch64-darwin
✅ 1 package built:
- duti
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-24.11
git worktree add -d .worktree/backport-357745-to-release-24.11 origin/release-24.11
cd .worktree/backport-357745-to-release-24.11
git switch --create backport-357745-to-release-24.11
git cherry-pick -x ed8e72640cb29a8792e99b7aabc740ccd7ebb7fb |
|
Could you handle the backport? |
Uses apple-sdk_11, sets host configuration flag based on host architecture, and nixfmt the package file.
Part of #354146.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.