Skip to content

Add link dependencies #50

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

Merged
merged 1 commit into from
Mar 13, 2025
Merged

Add link dependencies #50

merged 1 commit into from
Mar 13, 2025

Conversation

kingst
Copy link
Contributor

@kingst kingst commented Mar 11, 2025

This PR adds build dependencies to enable Xcode's parallel build system, used in Trio-dev (and others). The new dependencies are:

  • OmniKit: RileyLinkKit, RileyLinkBLEKit
  • OmniKitUI: OmniKit, RileyLinkKitUI

I tested this in Trio-dev (build only)

@marionbarker
Copy link
Collaborator

marionbarker commented Mar 11, 2025

This is one of 5 PR required to modify the build order scheme for LoopWorkspace from Build Order: Manual Order (which is deprecated and will be removed soon) to Build Order: Dependency Order.

All 5 of these repositories for LoopWorkspace must be updated to enable modification of the xcscheme from Build Order: Manual Order to Build Order: Dependency Order. After the modification, either build order works.

These are the modified SHA used for the successful build tests for LoopWorkspace.

+f1b09f35aa1ab8dd1655d4ba84bcf9563a38ecbb G7SensorKit (remotes/kingst/main)
+3add2bb644afe035cb785434cf3704a500c97068 LibreTransmitter (remotes/kingst/main)
+3ee3c6f68ca55804db226d4a8e639ccd6fcc8578 NightscoutRemoteCGM (remotes/kingst/add-loopkitui-dependency)
+05683a2689a2ef414e7e5b65f8763a9d31c6d1a0 OmniKit (remotes/kingst/main)
+65e23e28c834c6c5556548c66f8a98ed7855941c RileyLinkKit (remotes/kingst/add-loopkitui-dependency)

This was tested with both Xcode build and Browser Build.

Thanks to @kingst for his work in proposing these PR.

This particular change for OmniKit has been tested for both LoopWorkspace and for Trio-dev.

@marionbarker marionbarker requested review from ps2 and itsmojo March 11, 2025 23:11
@itsmojo
Copy link
Contributor

itsmojo commented Mar 13, 2025

The added dependencies to OmniKit in this PR look good to me, but I still can't a Loop Workspace to successfully build using Dependency Order for the LoopWorkspace Scheme with all 5 of the suggested PR's after many attempts. I believe the problem is elsewhere (for now at least) as the build failures don't appear to be related to the added dependences in this PR which should be added.

Copy link
Contributor

@itsmojo itsmojo left a comment

Choose a reason for hiding this comment

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

These updates seem reasonable. It would be good to be able to eventually build the LoopWorkspace using Dependency Order and these changes are a step towards that goal.

@kingst
Copy link
Contributor Author

kingst commented Mar 13, 2025

@itsmojo do you mind sharing the build errors? The errors are non-deterministic so my guess is that there is another dependency we missed

@marionbarker
Copy link
Collaborator

I confirmed that I could build LoopWorkspace with both Manual and Dependency build order when the SHA noted in this comment were used. The 5 PR are explicitly:

Merging these PR are a predecessor to selecting Dependency Build order in the Workspace; but do not require that change (which is why I tested both build orders).

I did run into difficulties with some Mac-Xcode testing of Dependency Build Order for the Amplitude plugin that was resolved by quitting and restarting Xcode. (Configuration: starting from LoopWorkspace main, editing the Scheme build order and adding those PR). The Manual Build Order always worked. The Browser Build worked, for both Manual and Dependency order - repeated several times.

I also tested the PR changes with (private) Trio-dev workspace which has Dependency Build Order. (Trio-dev does not use the NightscoutRemoteCGM submodule or Amplitude). This was successful with Mac-Xcode and Browser Build.

@marionbarker marionbarker merged commit 92948a7 into LoopKit:main Mar 13, 2025
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.

3 participants