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 bug detecting configurations from workspace #22069

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

Conversation

jaysoffian
Copy link
Contributor

@jaysoffian jaysoffian commented Jun 5, 2024

When enumerating the available configurations starting from an xcworkspace, we have to interpret the xcodeproj file references relative to the path of the xcworkspace. The code already handles this properly in the project_paths method, but for some reason doesn't qualify the paths correctly in the configurations method.

This has likely gone silently undetected because the bug only manifests itself when the xcworkspace is in a different directory from fastlane's CWD.

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.
  • I've added or updated relevant unit tests.

Motivation and Context

When enumerating the available configurations starting from an xcworkspace, we have to interpret the xcodeproj file references relative to the path of the xcworkspace. The code already handles this properly in the project_paths method, but for some reason doesn't qualify the paths correctly in the configurations method.

Description

Qualify xcodeproj paths relative to xcworkspace in the configurations method.

Testing Steps

Tested locally. We're running this patch in production at $dayjob. Also updated the unit test expectations to test correct behavior. They were testing for the buggy behavior.

@jaysoffian jaysoffian force-pushed the fix-bug-detecting-configuraitons-from-workspace branch 2 times, most recently from 69f5af3 to 326788c Compare June 5, 2024 20:46
When enumerating the available configurations starting from an xcworkspace, we
have to interpret the xcodeproj file references relative to the path of the
xcworkspace. The code already handles this properly in the `project_paths`
method, but for some reason doesn't qualify the paths correctly in the
`configurations` method.

This has likely gone silently undetected because the bug only manifests itself
when the xcworkspace is in a different directory from fastlane's CWD.
@jaysoffian jaysoffian force-pushed the fix-bug-detecting-configuraitons-from-workspace branch from 326788c to a933c76 Compare June 5, 2024 20:57
@jaysoffian
Copy link
Contributor Author

jaysoffian commented Jun 6, 2024

I wasn't sure why we hadn't run into this before. It's a really subtle bug. So here's what happened. We had a repo setup as follows:

Example/Example.xcworkspace
Example/Example.xcodeproj  # Configurations Foo, Bar, Baz

Someone then added a new top-level project so it looked like this:

Example/Example.xcworkspace
Example/Example.xcodeproj  # Configurations Foo, Bar, Baz
Example.xcodeproj # Configurations Foo, Bar

They then tried to build Example/Example.xcworkspace with configuration Baz. But fastlane resolved the reference to the top-level project which doesn't have any Baz configuration, complained the configuration was invalid, and ran xcodebuild without any -configuration setting which caused Xcode to use the default configuration which then failed to build because a bunch of other settings were at that point inconsistent.

The reason this worked before the top-level Example.xcodeproj was added was due to this additional bit of code in gym:

    # Detects the available configurations (e.g. Debug, Release)
    def self.detect_configuration
      config = Gym.config
      configurations = Gym.project.configurations
      return if configurations.count == 0 # this is an optional value anyway

      if config[:configuration]
        # Verify the configuration is available
        unless configurations.include?(config[:configuration])
          UI.error("Couldn't find specified configuration '#{config[:configuration]}'.")
          config[:configuration] = nil
        end
      end
    end

So previous to adding the top-level Example.xcodeproj, it wasn't able to find any configuration due to not resolving the project relative to the workspace, and so detect_configuration threw up its hands and returned (configurations.count == 0). But as soon as any configurations were returned, it decided the configuration specified on the command line must not be valid and proceeded to totally break the build.

So while this PR fixes the immediate bug, I now question the behavior of this detect_configuration method in the first place. I don't really see it providing anything of value.

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

Successfully merging this pull request may close these issues.

None yet

1 participant