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

Move xcodeproj folder away from the project root #1990

Open
mokagio opened this issue Aug 6, 2024 · 8 comments
Open

Move xcodeproj folder away from the project root #1990

mokagio opened this issue Aug 6, 2024 · 8 comments
Labels
Improvements [Priority] Low A less urgent issue that can be addressed when time allows.

Comments

@mokagio
Copy link
Contributor

mokagio commented Aug 6, 2024

When called in the root of the project xcodebuild -resolvePackageDependencies will generate Package.resolved in both the xcodeproj and xcworkspace folders. This is likely a bug, as the same command with the 16.0 beta 3 version only generates the resolved file in the xcodeproj folder.

Regardless of the cause, this behavior can, sometimes, result in CI failures such as the one in this build: https://buildkite.com/automattic/pocket-casts-ios/builds/7634#019113ca-d4bd-4838-b93a-aae0e4a0528f

  an out-of-date resolved file was detected at /opt/ci/builds/builder/automattic/pocket-casts-ios/podcasts.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved, which is not allowed when automatic dependency resolution is disabled; please make sure to update the file to reflect the changes in dependencies. Running resolver because requirements have changed.

One solution is to specify the workspace to the command. See how this build passes: https://buildkite.com/automattic/pocket-casts-ios/builds/7657

xcodebuild -resolvePackageDependencies \
  -workspace podcasts.xcworkspace \
  -scheme pocketcasts

While this approach does the job, it requires us to drop the SPM caching because the install_swift_dependencies command in the CI toolkit does not support those options.

Of course, we could add support at the toolkit level, but there is an alternative approach.

A different approach would be to move the xcodeproj in a subfolder. This is the setup used in WordPress and WooCommerce, for example.

I find this approach attractive because it would give us a chance to tidy up the project root folder. For example, we could use this structure:

Modules/  # already exists and contains Swift packages
Targets/ # would contain the sources for the various targets - each target could have its set of xcconfig
Xcode/
  Config/
    Project.base.xcconfig
    Project.debug.xcconfig
    # etc
  podcasts.xcodeproj/
# all the other files that stay in the root

@Automattic/apps-infrastructure what do you think? Is it worth establishing a convention of moving the xcodeproj folder away from the root if an xcworskpace is in use? Or should we take this as the occasion to make the install_swift_dependencies CI toolkit plugin more flexible?

Of course, nothing stops us from doing both, but give each approach would make the other unnecessary, I'd rather take a choice on which one to focus on in the short term.

@AliSoftware
Copy link
Contributor

@Automattic/apps-infrastructure what do you think? Is it worth establishing a convention of moving the xcodeproj folder away from the root if an xcworskpace is in use? Or should we take this as the occasion to make the install_swift_dependencies CI toolkit plugin more flexible?

While moving the xcodeproj file in a subfolder could be an easy fix but also a nice way to keep the repo root cleaner (and avoid new devs to accidentally open the xcodeproj instead of the xcworkspace), I think it would be better to provide a long-term solution that would work on all of our products and repos, without requiring them to have a specific file structure to work.

(also because, otherwise, how would we guarantee that someone wouldn't decide to move the xcodeproj file back to the repo root in the future without realizing such an apparently innocuous change would break SPM?)

TL;DR

I got some brainstorm about it below, but the TL;DR is that I think it'd make more sense to pass --workspace … --scheme … to the xcodebuild command in the hope that this would mimic Xcode UI's "Resolve Package Dependencies" behavior the closest, and thus have both local and CI be guaranteed to behave the same wrt to which Package.resolved to use/update.

That being said, despite the --scheme being a required parameter when you pass --workspace, I wonder if the scheme we pass makes any difference in practice (after all, the list of dependencies is defined at each xcodeproj's level, not at a scheme level), so maybe we can try to be clever and guess a --scheme to use in our invocation within install_swiftpm_dependencies?

Braindump of alternative ideas

Here is a braindump of a couple of alternative ideas / potential solutions I could think of when I thought about this problem:

1️⃣ See if creating a symlink of one Package.resolved to the other one could work

  • If it does, that means that even if Xcode / xcodebuild sometimes updates one and sometimes the other and we still end up with two sources of truth due to this Xcode quirk and inconsistency, at least we can guarantee that the 2 copies will always be consistent and in sync.
  • That being said, while I haven't tested it, I'm not super confident that it would work, as I wouldn't be surprised to see Xcode overwrite the files (and thus delete the symlink to recreate a regular Package.resolved file in its place) when it updates the dependencies

2️⃣ Add some logic to our install_swift_dependencies to sort out the Package.resolved files before calling the xcodebuild … command

  • We first need to determine with certainty which one to use as the source of truth. When the two files differ, should we copy the one from the xcworkspace into the xcodeproj, or the other way around? Your issue description mention that Xcode 16.0b3 only updates the one in xcodeproj, so that suggests this is the real source of truth to use? But I have my doubts about this because I'd instead expect the xcworkspace to be the one to use in this case—especially in case your workspace references multiple xcodeproj files). Also, what does Xcode's UI do when you ask to resolve SPM dependencies using the UI while the workspace is open in Xcode?
  • Once we know which one to use as the SoT, we could decide to cp that one to the other place, to guarantee that both would always be the same and the other one's content would always match the up-to-date resolved packages
  • Another thing we could try instead is to rm the other one that we consider not being the real SoT, so that xcodebuild only have one Package.resolved file to consider instead of two and picking the wrong one. But not sure if that would work, vs xcodebuild complaining it couldn't find the Package.resolved because it'd only look in the wrong place (instead of trying multiple places and pick the first one it finds)

3️⃣ Maybe we should pass --project to the xcodebuild command we call from install_swift_dependencies?

…instead of passing nothing, or of passing --workspace but also having to pass --scheme?

That would only make sense if the real SoT to consider—as suggested by Xcode 16.0b3's behavior, apparently?—is the xcodeproj and not the xcworkspace.

4️⃣ Make our install_swift_dependencies utility support additional parameters like scheme… or guess it?

Basically what you suggested above. Only make sense if the xcworkspace is the SoT to use.

Optionally, maybe there could be a way for us to try to be smart if the scheme is not passed explicitly, e.g. maybe we could use xcodebuild command to list all the schemes present and call xcodebuild with each of them in a loop… or just use the first one (as, despite the --scheme parameter being required by xcodebuild when also providing --workspace, in practice I'm not sure it matters with scheme we use when it comes to resolving Package Dependencies, given those dependencies are declared at project level and don't depend on the scheme selected in the UI… right?) 🤷


What's the real Source of Truth?

To be honest I'm not sure which one of the two makes more sense to be the Source of Truth.

  • On one end, it makes more sense for the xcworkspace to be the SoT because that's what we open and that's what references the underlying xcodeproj project(s) we work on. If we open a workspace referencing multiple projects and ask Xcode's UI to Resolve Package Dependencies, surely it resolves those once for the whole workspace at once?
  • On the other end, the list of package dependencies is located in the xcodeproj files themselves, since in Xcode's UI you have to select an Xcode project to then open the "Package Dependencies" tab. Try to think of what happens to workspaces like DayOne for example, which references both an iOS xcodeproj and a Mac xcodeproj, each having potentially quite different list of dependencies (and even if some dependencies are the same, nothing in Xcode UI prevents them to be specified with potentially different version requirements and dependency rules…)

Overall I'm not even sure what Xcode (UI) does when it tries to resolve package dependencies on a workspace:

  • Aggregate all the package rules of all its underlying xcodeproj files and resolve them as a unified list of dependencies (as if they were all defined in a single Package.swift) and generate a unique Package.resolved file in the xcworkspace (but then how would it handle potential conflicting dependency declarations in different xcodeproj files?)?
  • Or loop over each xcodeproj and resolve dependencies for each of them in turn independently (like if each had their own independant Package.swift) and generate independent Package.resolved in each xcodeproj project as a result?

But whatever Xcode UI's does, in the end what probably makes the most sense is to try to make our invocation of xcodebuild … on CI behave the exact same as when Xcode's own UI does a "Resolve Package Dependencies" step after you open the workspace in the UI. Which might involve providing the --workspace parameter explicitly to the xcodebuild command.

@AliSoftware
Copy link
Contributor

Funnily enough, xcodebuild --help suggests that one is supposed to be able to specify -workspace without -scheme when using -resolvePackageDependencies

$ xcodebuild --help
Usage: …
       xcodebuild -resolvePackageDependencies [-project <projectname>|-workspace <workspacename>] -clonedSourcePackagesDirPath <path>

But, as you already noticed, in practice calling that suggested invocation in practice ends up exiting with xcodebuild: error: If you specify a workspace then you must also specify a scheme. Use -list to see the schemes in this workspace. (at least in Xcode 15.4) 😒

@mokagio
Copy link
Contributor Author

mokagio commented Aug 6, 2024

in practice calling that suggested invocation in practice ends up exiting with xcodebuild: error: If you specify a workspace then you must also specify a scheme. Use -list to see the schemes in this workspace. (at least in Xcode 15.4) 😒

Same in Xcode 16.0

image

@mokagio
Copy link
Contributor Author

mokagio commented Aug 6, 2024

Allowing callers to forward -workspace + -scheme and/or -project to install_swift_dependencies (i.e. using the same "allow only a subset of flags to be passed for security and controlled usage, like we already do with other scripts) might be the options that gives us the most flexibility.

  • I won't require any change to setups where xcodebuild makes the right choice (like WordPress iOS)
  • It will allow this project to re-introduce caching without changing the project and folder structure
  • It will allow future projects to use whichever setup they prefer

@mokagio
Copy link
Contributor Author

mokagio commented Aug 6, 2024

...It would also save us from reverse engineer / understand the various Xcode / xcodebuild quirks...

@AliSoftware
Copy link
Contributor

AliSoftware commented Aug 6, 2024

...It would also save us from reverse engineer / understand the various Xcode / xcodebuild quirks...

True… though I'd argue we'd still benefit from understanding which xcodebuild … command matches the Xcode UI's behavior the closest (i.e. does Xcode UI do xcodebuild -workspace … -resolvePackageDependencies or xcodebuild -project … -resolvePackageDependencies under the hood when we select "Resolve Package Dependencies" from the File menu?), and then prefer using that for future projects or projects like PCiOS (where it doesn't do the right thing by default with our current implementation)

Besides, we still kinda need to understand what Xcode vs xcodebuild does, and in particular which of the two Package.resolved it relies on when we'll invoke our xcodebuild -resolvePackageDependencies … command from install_swiftpm_dependencies… because we compute the cache key from the hash of the Package.resolved file. So if we compute the hash from one Package.resolved but the xcodebuild command we use ends up updating the other one, the cache logic would be incorrect and break.

@AliSoftware
Copy link
Contributor

AliSoftware commented Aug 6, 2024

As a side note, I noticed that our current implementation of install_swiftpm_dependencies assumes that the xcworkspace is at the root of the repo, and that there's only one xcworkspace in total there (or at least always pick the first one even if there are more than one

While it's likely currently the case for most if not all our current projects (?)… I don't see any reason why we should really assume this to always be true; e.g. imagine a monorepo hosting multiple apps and where we'd have decided to put the xcworkspace of each app in their respective subfolders (and none at the root). Or even if we decided to keep the various xcworkspace files at the root, there would be more than one there…

All that is one more point in the camp of allowing install_swiftpm_dependencies to accept -workspace and -scheme parameters to be passed along, I guess, instead of having it try to guess it magically (and being right most of the time… but not always)

@AliSoftware
Copy link
Contributor

AliSoftware commented Aug 6, 2024

@mokagio I've started a draft implementation of some improvements to our install_swiftpm_dependencies utility in Automattic/a8c-ci-toolkit-buildkite-plugin#105. This is still WIP but curious about your thoughts on the approach and your configuration it would help solve the issue here in PCiOS.

I actually have some hope about a solution that would allow us to use -workspace without having to also specify -scheme (TL;DR: pass -list instead), which would solve the whole issue in mostly a transparent way (given the new auto-detect logic of the xcworkspace on the repo this utility is invoked as been improved too which means that in 95% of cases this would work transparently without having to pass --workspace to install_swiftpm_dependencies explicitly either…?). That theory needs to be thoroughly tested first though, but I'm at my EOD so won't test it today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvements [Priority] Low A less urgent issue that can be addressed when time allows.
Projects
None yet
Development

No branches or pull requests

3 participants