-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix: SPM correct platform for iOS #38
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,3 +113,29 @@ export function getMajoriOSVersion(config: Config): string { | |
| ); | ||
| return iosVersion; | ||
| } | ||
|
|
||
| export function getMajorMinoriOSVersion(config: Config): string { | ||
| const pbx = readFileSync(join(config.ios.nativeXcodeProjDirAbs, 'project.pbxproj'), 'utf-8'); | ||
| const searchString = 'IPHONEOS_DEPLOYMENT_TARGET = '; | ||
| const startIndex = pbx.indexOf(searchString); | ||
| if (startIndex === -1) { | ||
| return ''; | ||
| } | ||
| const valueStart = startIndex + searchString.length; | ||
| // Extract until semicolon or newline (typical end of value in pbxproj) | ||
| const endIndex = pbx.indexOf(';', valueStart); | ||
| const newlineIndex = pbx.indexOf('\n', valueStart); | ||
| const actualEnd = endIndex !== -1 && newlineIndex !== -1 | ||
| ? Math.min(endIndex, newlineIndex) | ||
| : endIndex !== -1 | ||
| ? endIndex | ||
| : newlineIndex !== -1 | ||
| ? newlineIndex | ||
| : pbx.length; | ||
| let iosVersion = pbx.substring(valueStart, actualEnd).trim(); | ||
| // Remove trailing .0 if present | ||
| if (iosVersion.endsWith('.0')) { | ||
| iosVersion = iosVersion.slice(0, -2); | ||
| } | ||
| return iosVersion; | ||
| } | ||
|
Comment on lines
+117
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review the empty string return behavior and address the pipeline failure. The function logic for extracting the iOS deployment target is sound, but there are a few concerns:
Optional refactor to reduce duplication: The logic overlaps significantly with export function getMajoriOSVersion(config: Config): string {
const fullVersion = getMajorMinoriOSVersion(config);
return fullVersion.split('.')[0] || fullVersion;
}This would maintain backward compatibility while eliminating duplication. 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,7 @@ import { extract } from 'tar'; | |||||||||||||||||||||||||||||||||||||||
| import { getCapacitorPackageVersion } from '../common'; | ||||||||||||||||||||||||||||||||||||||||
| import type { Config } from '../definitions'; | ||||||||||||||||||||||||||||||||||||||||
| import { fatal } from '../errors'; | ||||||||||||||||||||||||||||||||||||||||
| import { getMajoriOSVersion } from '../ios/common'; | ||||||||||||||||||||||||||||||||||||||||
| import { getMajorMinoriOSVersion } from '../ios/common'; | ||||||||||||||||||||||||||||||||||||||||
| import { logger } from '../log'; | ||||||||||||||||||||||||||||||||||||||||
| import type { Plugin } from '../plugin'; | ||||||||||||||||||||||||||||||||||||||||
| import { getPluginType, PluginType } from '../plugin'; | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -92,15 +92,15 @@ export async function removeCocoapodsFiles(config: Config): Promise<void> { | |||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| export async function generatePackageText(config: Config, plugins: Plugin[]): Promise<string> { | ||||||||||||||||||||||||||||||||||||||||
| const iosPlatformVersion = await getCapacitorPackageVersion(config, config.ios.name); | ||||||||||||||||||||||||||||||||||||||||
| const iosVersion = getMajoriOSVersion(config); | ||||||||||||||||||||||||||||||||||||||||
| const iosVersion = getMajorMinoriOSVersion(config); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| let packageSwiftText = `// swift-tools-version: 5.9 | ||||||||||||||||||||||||||||||||||||||||
| import PackageDescription | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // DO NOT MODIFY THIS FILE - managed by Capacitor CLI commands | ||||||||||||||||||||||||||||||||||||||||
| let package = Package( | ||||||||||||||||||||||||||||||||||||||||
| name: "CapApp-SPM", | ||||||||||||||||||||||||||||||||||||||||
| platforms: [.iOS(.v${iosVersion})], | ||||||||||||||||||||||||||||||||||||||||
| platforms: [.iOS(${iosVersion.includes('.') ? `"${iosVersion}"` : `.v${iosVersion}`})], | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+95
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle empty version string to prevent invalid Swift code generation. If Consider adding a fallback or validation: const iosVersion = getMajorMinoriOSVersion(config);
+ if (!iosVersion) {
+ fatal('Unable to determine iOS deployment target from project.pbxproj');
+ }
let packageSwiftText = `// swift-tools-version: 5.9Alternatively, provide a sensible default: - const iosVersion = getMajorMinoriOSVersion(config);
+ const iosVersion = getMajorMinoriOSVersion(config) || '13';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
| products: [ | ||||||||||||||||||||||||||||||||||||||||
| .library( | ||||||||||||||||||||||||||||||||||||||||
| name: "CapApp-SPM", | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: Cap-go/capacitor-plus
Length of output: 88
🏁 Script executed:
head -150 ./cli/src/ios/common.ts | tail -20Repository: Cap-go/capacitor-plus
Length of output: 733
🏁 Script executed:
sed -n '100,160p' ./cli/src/ios/common.tsRepository: Cap-go/capacitor-plus
Length of output: 1730
🏁 Script executed:
Repository: Cap-go/capacitor-plus
Length of output: 49
🏁 Script executed:
Repository: Cap-go/capacitor-plus
Length of output: 456
🏁 Script executed:
Repository: Cap-go/capacitor-plus
Length of output: 47
🏁 Script executed:
rg "getMajorMinoriOSVersion|getMajoriOSVersion" --type tsRepository: Cap-go/capacitor-plus
Length of output: 705
🏁 Script executed:
sed -n '1,50p' ./cli/src/util/spm.tsRepository: Cap-go/capacitor-plus
Length of output: 1939
🏁 Script executed:
grep -n -A 5 "getMajorMinoriOSVersion" ./cli/src/util/spm.tsRepository: Cap-go/capacitor-plus
Length of output: 552
🏁 Script executed:
sed -n '90,130p' ./cli/src/util/spm.tsRepository: Cap-go/capacitor-plus
Length of output: 1627
🏁 Script executed:
Repository: Cap-go/capacitor-plus
Length of output: 47
🏁 Script executed:
sed -n '100,110p' ./cli/src/util/spm.tsRepository: Cap-go/capacitor-plus
Length of output: 508
Trailing .0 removal logic is correct; add test coverage for version formats.
The logic properly handles iOS version formatting for SPM platform syntax:
15.0→15(uses.v15enum syntax)15.5→15.5(uses"15.5"string syntax)15.5.0→15.5(uses"15.5"string syntax)This function lacks test coverage. Add tests to the test suite verifying these version formats are correctly extracted and normalized:
🤖 Prompt for AI Agents