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.

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

fix: cleanup xcode_backend.sh to fix iOS build w/ NixOS/nixpkgs flutter #155139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

phlip9
Copy link

@phlip9 phlip9 commented Sep 13, 2024

Overview

This patch cleans up the packages/flutter_tools/bin/xcode_backend.sh wrapper. All it does now is (morally):

exec $FLUTTER_ROOT/bin/dart ./xcode_backend.dart

Xcode calls this wrapper during an iOS build. The previous xcode_backend.sh tries to discover $FLUTTER_ROOT from argv[0], even though its presence is already guaranteed (the wrapped xcode_backend.dart in fact relies on this env existing).

This $FLUTTER_ROOT logic then breaks when run using flutter packaged by nixpkgs.

Extra context

See also: NixOS/nixpkgs#341470

When using nixpkgs flutter, the flutter SDK directory is composed of several immutable layers, joined together using symlinks (called a symlinkJoin). Without this patch, the auto-discover traverses the symlinks into the wrong layer, and so it uses an "unwrapped" dart command instead of a "wrapped" dart that sets some important envs/flags (like $FLUTTER_ROOT).

See: https://github.com/NixOS/nixpkgs/blob/6ec57b76a9a280f4f99bd0dca1e4ab4880fc02c4/pkgs/development/compilers/flutter/flutter.nix#L126-L134

Using the "unwrapped" dart then manifests in this error when compiling, since it doesn't see the ios build-support artifacts:

$ flutter run -d iphone
Running Xcode build...
Xcode build done.                                            6.4s
Failed to build iOS app
Error (Xcode): Target debug_unpack_ios failed: Error: Flutter failed to create a directory at "/nix/store/xhcdmzj0m8x64fky1mdsyv5311f2sxs9-flutter-3.24.1-unwrapped/bin/cache/artifacts".

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

…tter

This patch cleans up `xcode_backend.sh`. It now effectively just runs
`exec $FLUTTER_ROOT/bin/dart ./xcode_backend.dart`.

The previous `xcode_backend.sh` tries to discover `$FLUTTER_ROOT` from
argv[0], even though its presence is already guaranteed (the wrapped
`xcode_backend.dart` also relies on this env).

When using nixpkgs flutter, the flutter SDK directory is composed of several
layers, joined together using symlinks (called a `symlinkJoin`). Without this
patch, the auto-discover traverses the symlinks into the wrong layer, and so it
uses an "unwrapped" `dart` command instead of a "wrapped" dart that sets some
important envs/flags (like `$FLUTTER_ROOT`).

See: <https://github.com/NixOS/nixpkgs/blob/6ec57b76a9a280f4f99bd0dca1e4ab4880fc02c4/pkgs/development/compilers/flutter/flutter.nix#L126-L134>

Using the "unwrapped" dart then manifests in this error when compiling, since
it doesn't see the ios build-support artifacts:

```
$ flutter run -d iphone
Running Xcode build...
Xcode build done.                                            6.4s
Failed to build iOS app
Error (Xcode): Target debug_unpack_ios failed: Error: Flutter failed to create a directory at "/nix/store/xhcdmzj0m8x64fky1mdsyv5311f2sxs9-flutter-3.24.1-unwrapped/bin/cache/artifacts".
```
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant