[go: nahoru, domu]
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

Make macrobenchmarks buildable for macos #71760

Merged
merged 1 commit into from
Dec 5, 2020
Merged

Make macrobenchmarks buildable for macos #71760

merged 1 commit into from
Dec 5, 2020

Conversation

jmagman
Copy link
Member
@jmagman jmagman commented Dec 5, 2020

Description

$ cd dev/benchmarks/macrobenchmarks/
$ flutter build macos
...
error: /Users/magder/Projects/flutter/dev/benchmarks/macrobenchmarks/macos/Runner/Configs/Release.xcconfig:1: could not find included file '../../Flutter/Flutter-Release.xcconfig' in search paths (in target 'Flutter Assemble' from project 'Runner')

Flutter-Debug.xcconfig and Flutter-Release.xcconfig are supposed to be checked into the git repo. https://github.com/flutter/flutter/pull/48251/files should not have added these gitignore exceptions.

You can see these are correctly checked in for flutter_gallery:
https://github.com/flutter/flutter/tree/89ef88c64f4d4b4fe81d44174de0e0abb442817f/dev/integration_tests/flutter_gallery/macos/Flutter

Also, actually build it and let CocoaPods do its thing.

@jmagman jmagman added a: tests "flutter test", flutter_test, or one of our tests team Infra upgrades, team productivity, code health, technical debt. See also team: labels. platform-mac Building on or for macOS specifically labels Dec 5, 2020
@jmagman jmagman self-assigned this Dec 5, 2020
@google-cla google-cla bot added the cla: yes label Dec 5, 2020
stuartmorgan
stuartmorgan approved these changes Dec 5, 2020
View reviewed changes
Copy link
Contributor
@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

One more line needs removing, and that file should be added too, but otherwise LGTM.

.gitignore
@@ -101,9 +101,6 @@ unlinked_spec.ds

# macOS
**/macos/Flutter/GeneratedPluginRegistrant.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

This is wrong too. Only Flutter/ephemeral (which is in macos/.gitignore) is designed to be ignored on macOS. That way we can always add new files to it and control whether they are or are not checked in even for existing projects, to avoid issues I saw with mobile.

Copy link
Member Author
@jmagman jmagman Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think that file needs to move to ephemeral then, it is generated:

//
//  Generated file. Do not edit.
//

import FlutterMacOS
import Foundation


func RegisterGeneratedPlugins(registry: FlutterPluginRegistry) {
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed #71762, let's deal with that separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

For the reasons I gave in the bug, I am not confident that this PR will meet the started goal (having those be buildable on macOS) in all cases without fixing this.

Copy link
Member Author

Choose a reason for hiding this comment

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

This project, at least, is buildable on macOS with just this change.

@jmagman jmagman merged commit f72e9c7 into flutter:master Dec 5, 2020
@jmagman jmagman deleted the build-macrobenchmarks-macos branch December 5, 2020 02:50
xster
xster reviewed Dec 5, 2020
View reviewed changes
Copy link
Member
@xster xster left a comment

Choose a reason for hiding this comment

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

LG

dev/benchmarks/macrobenchmarks/macos/Flutter/Flutter-Debug.xcconfig
@@ -0,0 +1,2 @@
#include "Pods/Target Support Files/Pods-Runner/Pods-Runner.debug.xcconfig"
#include "ephemeral/Flutter-Generated.xcconfig"
Copy link
Member

Choose a reason for hiding this comment

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

I don't really know how desktop works so I might be asking dumb things. What's the macos/Flutter/ephemeral/flutter_export_environment.sh file for? I don't see it referenced anywhere.

Copy link
Member Author
@jmagman jmagman Dec 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think I just never special cased that code to not generate the script for macOS. It would (someday) be used for macOS add-to-app though. It's unused right now.

Copy link
Member

Choose a reason for hiding this comment

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

ah ok. good to know. I was trying to figure out where it plugged in :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan stuartmorgan stuartmorgan approved these changes

@xster xster xster left review comments

Assignees

@jmagman jmagman

Labels
a: tests "flutter test", flutter_test, or one of our tests platform-mac Building on or for macOS specifically team Infra upgrades, team productivity, code health, technical debt. See also team: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jmagman @stuartmorgan @xster