Skip to content

Commit 3e3cc6f

Browse files
domenkozarclaude
andcommitted
refactor(php): dedup library-name resolution and simplify the SDK
Give Native one authority for the platform cdylib file name and have the ext-ffi installer reuse it, so the downloaded copy and the loader can no longer drift. Collapse the list-of-one library-name lookup, borrow the request JSON as &str across the native boundary, and factor the repeated builder wiring and request-field setters into shared helpers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 67415c7 commit 3e3cc6f

5 files changed

Lines changed: 62 additions & 59 deletions

File tree

secretspec-php/native/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ use ext_php_rs::prelude::*;
1818
///
1919
/// Exposed to PHP as the global function `secretspec_native_resolve()`.
2020
#[php_function]
21-
pub fn secretspec_native_resolve(request_json: String) -> String {
22-
secretspec::resolve_json(&request_json)
21+
pub fn secretspec_native_resolve(request_json: &str) -> String {
22+
secretspec::resolve_json(request_json)
2323
}
2424

2525
/// The extension's version (tracks the crate version). Exposed to PHP as

secretspec-php/scripts/install-ffi-lib.php

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,17 @@ function note(string $message): never
5656
note('SECRETSPEC_FFI_LIB is set; skipping download.');
5757
}
5858

59-
// Map the running platform to the release target triple + library file name.
60-
[$target, $libName] = secretspec_target();
59+
// Map the running platform to the release target triple; the library file name
60+
// comes from Native so the download target and the loader agree on one name.
61+
$target = secretspec_target();
6162
if ($target === null) {
6263
note('no prebuilt secretspec-ffi library for this platform; set '
6364
. 'SECRETSPEC_FFI_LIB or install the secretspec extension.');
6465
}
66+
if (!class_exists(\Secretspec\Native::class)) {
67+
note('the Secretspec classes are not autoloadable; run `composer install` first.');
68+
}
69+
$libName = \Secretspec\Native::libraryFileName();
6570

6671
$libDir = dirname(__DIR__) . '/lib';
6772
$dest = $libDir . '/' . $libName;
@@ -106,10 +111,10 @@ function note(string $message): never
106111
exit(0);
107112

108113
/**
109-
* @return array{0: ?string, 1: string} the release target triple (or null if
110-
* unsupported) and the platform library file name
114+
* The release target triple for the running platform, or null if no prebuilt
115+
* secretspec-ffi library is published for it.
111116
*/
112-
function secretspec_target(): array
117+
function secretspec_target(): ?string
113118
{
114119
$machine = strtolower(php_uname('m'));
115120
$isArm = in_array($machine, ['arm64', 'aarch64'], true);
@@ -118,25 +123,25 @@ function secretspec_target(): array
118123
switch (PHP_OS_FAMILY) {
119124
case 'Linux':
120125
if ($isX64) {
121-
return ['x86_64-unknown-linux-gnu', 'libsecretspec_ffi.so'];
126+
return 'x86_64-unknown-linux-gnu';
122127
}
123128
if ($isArm) {
124-
return ['aarch64-unknown-linux-gnu', 'libsecretspec_ffi.so'];
129+
return 'aarch64-unknown-linux-gnu';
125130
}
126131
break;
127132
case 'Darwin':
128133
if ($isArm) {
129-
return ['aarch64-apple-darwin', 'libsecretspec_ffi.dylib'];
134+
return 'aarch64-apple-darwin';
130135
}
131136
break;
132137
case 'Windows':
133138
if ($isX64) {
134-
return ['x86_64-pc-windows-msvc', 'secretspec_ffi.dll'];
139+
return 'x86_64-pc-windows-msvc';
135140
}
136141
break;
137142
}
138143

139-
return [null, ''];
144+
return null;
140145
}
141146

142147
/** The installed version of this package, or null in a dev/path checkout. */
@@ -163,11 +168,11 @@ function secretspec_installed_version(): ?string
163168
/** GET a URL, returning the body or null on any failure. */
164169
function secretspec_fetch(string $url): ?string
165170
{
171+
// PHP's HTTP stream wrapper reads its options from the 'http' key for both
172+
// http:// and https:// URLs, so a single entry covers both.
166173
$context = stream_context_create([
167174
'http' => ['method' => 'GET', 'follow_location' => 1, 'timeout' => 30,
168175
'header' => 'User-Agent: secretspec-php-installer'],
169-
'https' => ['method' => 'GET', 'follow_location' => 1, 'timeout' => 30,
170-
'header' => 'User-Agent: secretspec-php-installer'],
171176
]);
172177
$body = @file_get_contents($url, false, $context);
173178

secretspec-php/src/Builder.php

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,38 +29,32 @@ final class Builder
2929
/** Path to a `secretspec.toml`; omit to walk up from the working directory. */
3030
public function withPath(?string $path): self
3131
{
32-
if ($path !== null) {
33-
$this->request['path'] = $path;
34-
}
35-
36-
return $this;
32+
return $this->set('path', $path);
3733
}
3834

3935
/** Provider address, e.g. `keyring://` or `dotenv://.env.production`. */
4036
public function withProvider(?string $provider): self
4137
{
42-
if ($provider !== null) {
43-
$this->request['provider'] = $provider;
44-
}
45-
46-
return $this;
38+
return $this->set('provider', $provider);
4739
}
4840

4941
/** Profile to resolve, e.g. `production`. */
5042
public function withProfile(?string $profile): self
5143
{
52-
if ($profile !== null) {
53-
$this->request['profile'] = $profile;
54-
}
55-
56-
return $this;
44+
return $this->set('profile', $profile);
5745
}
5846

5947
/** Human-readable reason for the access, surfaced to reason-policy providers. */
6048
public function withReason(?string $reason): self
6149
{
62-
if ($reason !== null) {
63-
$this->request['reason'] = $reason;
50+
return $this->set('reason', $reason);
51+
}
52+
53+
/** Set a request field when the value is provided; a no-op for null. */
54+
private function set(string $key, ?string $value): self
55+
{
56+
if ($value !== null) {
57+
$this->request[$key] = $value;
6458
}
6559

6660
return $this;

secretspec-php/src/Native.php

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,12 @@ private static function locateLibrary(): string
125125
return $env;
126126
}
127127

128-
$names = self::libraryNames();
128+
$name = self::libraryFileName();
129129

130130
// A copy bundled alongside the package (distribution layout).
131-
foreach ($names as $name) {
132-
$bundled = \dirname(__DIR__) . \DIRECTORY_SEPARATOR . 'lib' . \DIRECTORY_SEPARATOR . $name;
133-
if (\is_file($bundled)) {
134-
return $bundled;
135-
}
131+
$bundled = \dirname(__DIR__) . \DIRECTORY_SEPARATOR . 'lib' . \DIRECTORY_SEPARATOR . $name;
132+
if (\is_file($bundled)) {
133+
return $bundled;
136134
}
137135

138136
// Walk up from the package looking for a Cargo target dir; pick the most
@@ -143,15 +141,13 @@ private static function locateLibrary(): string
143141
$best = null;
144142
$bestMtime = -1;
145143
foreach (['release', 'debug'] as $profile) {
146-
foreach ($names as $name) {
147-
$candidate = $dir . \DIRECTORY_SEPARATOR . 'target'
148-
. \DIRECTORY_SEPARATOR . $profile . \DIRECTORY_SEPARATOR . $name;
149-
if (\is_file($candidate)) {
150-
$mtime = \filemtime($candidate);
151-
if ($mtime !== false && $mtime > $bestMtime) {
152-
$best = $candidate;
153-
$bestMtime = $mtime;
154-
}
144+
$candidate = $dir . \DIRECTORY_SEPARATOR . 'target'
145+
. \DIRECTORY_SEPARATOR . $profile . \DIRECTORY_SEPARATOR . $name;
146+
if (\is_file($candidate)) {
147+
$mtime = \filemtime($candidate);
148+
if ($mtime !== false && $mtime > $bestMtime) {
149+
$best = $candidate;
150+
$bestMtime = $mtime;
155151
}
156152
}
157153
}
@@ -171,13 +167,17 @@ private static function locateLibrary(): string
171167
);
172168
}
173169

174-
/** @return list<string> platform-specific shared-library file names. */
175-
private static function libraryNames(): array
170+
/**
171+
* The platform-specific `libsecretspec_ffi` file name the loader looks for.
172+
* Shared with the `secretspec-install-lib` script so the downloaded copy and
173+
* the loader agree on one name.
174+
*/
175+
public static function libraryFileName(): string
176176
{
177177
return match (\PHP_OS_FAMILY) {
178-
'Darwin' => ['libsecretspec_ffi.dylib'],
179-
'Windows' => ['secretspec_ffi.dll'],
180-
default => ['libsecretspec_ffi.so'],
178+
'Darwin' => 'libsecretspec_ffi.dylib',
179+
'Windows' => 'secretspec_ffi.dll',
180+
default => 'libsecretspec_ffi.so',
181181
};
182182
}
183183
}

secretspec-php/src/SecretSpec.php

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,7 @@ public static function resolve(
4747
?string $profile = null,
4848
?string $reason = null,
4949
): Resolved {
50-
return self::builder()
51-
->withPath($path)
52-
->withProvider($provider)
53-
->withProfile($profile)
54-
->withReason($reason)
55-
->load();
50+
return self::configured($path, $provider, $profile, $reason)->load();
5651
}
5752

5853
/**
@@ -67,12 +62,21 @@ public static function report(
6762
?string $profile = null,
6863
?string $reason = null,
6964
): Report {
65+
return self::configured($path, $provider, $profile, $reason)->report();
66+
}
67+
68+
/** Build a {@see Builder} from the shared one-shot options. */
69+
private static function configured(
70+
?string $path,
71+
?string $provider,
72+
?string $profile,
73+
?string $reason,
74+
): Builder {
7075
return self::builder()
7176
->withPath($path)
7277
->withProvider($provider)
7378
->withProfile($profile)
74-
->withReason($reason)
75-
->report();
79+
->withReason($reason);
7680
}
7781

7882
/** The ABI version reported by the loaded native library. */

0 commit comments

Comments
 (0)