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 unstable scheme generation #790

Merged
merged 3 commits into from Dec 16, 2019

Conversation

marciniwanicki
Copy link
Collaborator

@marciniwanicki marciniwanicki commented Dec 15, 2019

Short description πŸ“

Running tuist generate second time without modifying the project can result in unexpected differences in project .xcscheme files.

Solution πŸ“¦

Sort both arguments and environment variables by name during the generation process.

Implementation πŸ‘©β€πŸ’»πŸ‘¨β€πŸ’»

  • Update integration tests to cover schemes generation
  • Sort arguments and environment variables
  • Update CHANGELOG

Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Nice work - the stable generation tests are really useful πŸ‘

@@ -572,7 +572,7 @@ final class SchemesGenerator: SchemesGenerating {
private func commandlineArgruments(_ arguments: [String: Bool]) -> [XCScheme.CommandLineArguments.CommandLineArgument] {
arguments.map { key, enabled in
XCScheme.CommandLineArguments.CommandLineArgument(name: key, enabled: enabled)
}
}.sorted { $0.name < $1.name }
Copy link
Contributor

Choose a reason for hiding this comment

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

Does order matter when generating schemes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering, why there are differences in order when generating subsequent times, can we identify the root cause of the problem?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Generally it does, but this change is not regressing as the order is currently indeterministic. Please check #791 for more details.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lakpa Arguments are stored as a dictionary in the manifest, that's why they're shuffled.

public struct Arguments: Equatable, Codable {
    public let environment: [String: String]
    public let launch: [String: Bool]

    public init(environment: [String: String] = [:],
                launch: [String: Bool] = [:]) {
        self.environment = environment
        self.launch = launch
    }
}

@pepicrft
Copy link
Contributor

Good to merge!

@pepicrft pepicrft merged commit 7cdb28e into tuist:master Dec 16, 2019
@adamkhazi
Copy link
Contributor

Nice fix πŸ‘

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants