Repository navigation
Take the win32 6 chain, which upstream made smaller than it looked - #497
Merged
Merged
Conversation
Four packages that do not move one at a time: `file_picker` 8.3.7 -> 13.3.3, `package_info_plus` 8.3.1 -> 10.2.2, `device_info_plus` 11.5.0 -> 13.3.0 and `share_plus` 12.0.2 -> 13.3.1. Each of the last three requires `win32 ^6.0.1` while `file_picker` 8.x pins `win32 ^5`, so the four are one change. `win32` goes 5.15.0 -> 6.4.0 and `win32_registry` 2.1.0 -> 3.0.3; `windows_file_picker` arrives, since 12.0.0 split the plugin per platform, and `flutter_plugin_android_lifecycle` drops out. The prerequisite was already done: our fork of `super_native_extensions` widened `device_info_plus` to `<14.0.0` in 059a6db, and `main` points at it. **Three of the four were free. `file_picker` crosses five majors**, and the reading that matters is what 12.0.0 did and what 2.1.1 undid. 12.0.0 made `pickFiles` answer `List<PlatformFile>` with no nullability, and every implementation maps the channel's null to `[]`. So a cancelled pick and a pick whose every chosen file was dropped would have become the same value - and this app reports those differently: #354 in the archive browser, and the firmware picker's warning. **But `android_file_picker` 2.1.1 fixed that upstream**, in its own words for #2262: "Picking several files now fails with `unknown_path` when none of them can be read, instead of returning an empty list as if the user cancelled." `FileUtils.kt`'s `handleFileResult` is now `if (files.isNotEmpty()) finishWithSuccess(files) else finishWithError("unknown_path", ...)`, and every result path goes through it. So on Android `[]` means a cancellation and nothing else, the all-dropped pick is a rejection, and the `catch` each call site already has reports it with the sentence #354 asked for. `picked.isEmpty` is a plain `return` again. `PlatformFile.size` is gone and nothing here used it: `picked_file.dart` already stats the path itself, deliberately, because the desktop pickers reported `0` for a path that no longer existed. `withData` and `withReadStream` are gone as parameters; the comments that named them now say what they meant. **Three pickers had to move to `pickFile`.** 12.0.0 defaulted `allowMultiple` to true and 13.0.0 removed it, so `pickFiles` picks several. The firmware archive, Pixel Draw's import and the plotter's were all single-select, and leaving them on `pickFiles` would have let a user choose two archives and then read `the picker returned 2 file(s) and no usable path`. `pickFile` is the API that still takes one; the archive browser keeps `pickFiles`, which is the one surface that means it. **`saveFile` answers a `Uri` now**, where it answered a path. Only a `file:` one names something `dart:io` can open, so the desktop write resolves it and returns early otherwise. Android is the platform that does not give one - its delegate replies with the chosen document's path component alone, which parses with an empty scheme - and it has already written the bytes itself. **No CHANGELOG entry, because nothing a user can observe changes.** I tried twice to find one and both attempts were wrong, which is the argument for not inventing a third: the first claimed an iOS ordering fix that belongs to the photo-library picker, not the document picker this app uses, and the second claimed the save no longer poisons the remembered folder - but `picker_memory.dart`'s `_remembers` is false on Android, so there is no remembered folder there to poison. The pickers look and behave as they did; the `file:` check is defensive rather than a fix. `folder_pick.dart`'s own doc asked whoever took this major to check two things, and both held: the channel name is unchanged and `dir` is still the method, on both platforms, and `getDirectoryPath` still swallows every `PlatformException` - so #353's workaround is still needed. What did **not** hold is the argument map, and it matters more than it looks: 13.x sends `androidSafOptions` for `dir`, and `FileUtils.kt` answers `data.data.toString()` - a `content://…/tree` URI - instead of the path `getFullPathFromTreeUri` resolves whenever that argument is non-null. Every caller here uses the answer as a path. The empty map this file has always sent is therefore load-bearing now rather than merely equal to the plugin's, and the app's own fall-back agrees with it: `FilePicker.getDirectoryPath`'s default is `AndroidOptions`, not `FilePickerAndroidOptions`, so it sends null for it too. That doc has been rewritten to say so, along with five claims the upgrade falsified - the `FilePickerPlugin.java` latch is fixed upstream, `file_picker_linux` 2.0.4 is an XDG portal with no `zenity`, and macOS does answer `dir`. The test side: * The seam moved from `FilePicker.platform` to `FilePickerPlatform.instance`, whose token a subclass inherits, so no mixin and no new dev dependency for it. `test/picker_fakes.dart` absorbed that and the four files that still had their own thirteen-parameter copy - which is what its own doc said this bump should do. * `dropped_pick_test.dart` now drives the rejection rather than an empty result, because that is what the platform sends. The three cases it had are still the three outcomes that differ. * `isPickerCall` in `test/ratchet.dart` follows the API onto the class. Two ratchets read it, and without the change both would have counted zero pickers and passed while claiming to have checked ten. * `folder_pick_test.dart`'s pinning case drives `FilePickerAndroid` where it drove `FilePickerIO`, with `const AndroidOptions()` - what the facade passes - rather than that class's own default, which would have pinned a call the app never makes. `android_file_picker` is a dev dependency for it. The channel is rebuilt with the default codec: 8.x's implementation used `JSONMethodCodec` and the federated ones do not. * `picker_bytes_guard_test.dart` guards two parameters that no longer exist, so it cannot be violated. Its header says so now rather than reading as a live rule, and names where the risk went instead. No budget rose, and both ratchets are still at zero everywhere. `device_info_plus` 13 does **not** close the disk-space gap b3 left open: it still reads `NSFileSystemSize` unconditionally and still ships an empty `NSPrivacyAccessedAPITypes`. The remaining remedy is the guard in our fork, and `test/privacy_manifest_test.dart`'s pinned finding is unchanged. Gates: `flutter analyze` clean, 2546 tests, `check_format.sh` clean, `check_changelog.sh` OK. **Not verified on a device**, and three of the changes are platform behaviour: the folder pick's arguments, the single-selection pickers and the save-file scheme check. Nothing in this project compiles Android or iOS on a pull request.
…ld not
`StubPickedFile` derived `path` by handing the fixture string to
`Uri.file` and letting `PlatformFile` read it back. That round trip is
host-dependent: `Uri.file('C:/tmp/fw.tgz')` is absolute on Windows and
*relative* on Linux, a relative file URI carries no scheme, and
`PlatformFile.path` answers null for any scheme but `file:`. So the fixture
named a file here and named nothing on the runner, and two
`firmware_double_start_test.dart` cases read `the picker returned fw.tgz with
no usable path` instead of installing once.
The stub now holds the string and answers it untouched, which is also what the
plugin does with what the platform sent, and `uri` joins the members that
throw because nothing under test reads it. The fixture is a POSIX path too, so
it no longer reads as Windows-only in a suite CI runs on Linux - though after
this it would work either way.
Local gates cannot see this class of defect: one host, one answer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four packages that do not move one at a time:
file_picker8.3.7 → 13.3.3,package_info_plus8.3.1 → 10.2.2,device_info_plus11.5.0 → 13.3.0 andshare_plus12.0.2 → 13.3.1. Each of the last three requireswin32 ^6.0.1while
file_picker8.x pinswin32 ^5, so the four are one change.win32goes 5.15.0 → 6.4.0 and
win32_registry2.1.0 → 3.0.3;windows_file_pickerarrives, since 12.0.0 split the plugin per platform, and
flutter_plugin_android_lifecycledrops out.This is d8 on the dependency plan. Its prerequisite was already done: our
fork of
super_native_extensionswideneddevice_info_plusto<14.0.0in059a6db and
mainalready points at it.Three of the four were free.
file_pickercrosses five majorsThe reading that matters is what 12.0.0 did and what
android_file_picker2.1.1 undid.
12.0.0 made
pickFilesanswerList<PlatformFile>with no nullability, andevery implementation maps the channel's
nullto[]. That would have made acancelled pick and a pick whose every chosen file was dropped the same value —
and this app reports those differently: #354 in the archive browser, and
the firmware picker's warning.
But upstream fixed it, in its own words for #2262:
FileUtils.kt'shandleFileResultis nowif (files.isNotEmpty()) finishWithSuccess(files) else finishWithError("unknown_path", …),and every result path goes through it. So on Android
[]means a cancellationand nothing else, the all-dropped pick is a rejection, and the
catcheachcall site already has reports it with the sentence #354 asked for.
picked.isEmptyis a plainreturnagain.That conclusion is the second draft. The first built a shim over the
plugin's private method channel to recover the distinction by hand, and review
found that the distinction no longer needs recovering. The shim is gone, along
with its two budget rises — both ratchets are back to zero everywhere.
Three pickers had to move to
pickFile12.0.0 defaulted
allowMultipleto true and 13.0.0 removed it, sopickFilespicks several. The firmware archive, Pixel Draw's import and the plotter's were
all single-select, and leaving them on
pickFileswould have let a user choosetwo archives and then read
the picker returned 2 file(s) and no usable path.pickFileis the API that still takes one. The archive browser keepspickFiles, which is the one surface that means it.The folder pick's empty argument map is now load-bearing
folder_pick.dart's own doc asked whoever took this major to check two things,and both held: the channel name is unchanged and
diris still the method, onboth platforms, and
getDirectoryPathstill swallows everyPlatformException— so #353's workaround is still needed.
What did not hold is the argument map, and that was nearly a bug of my own
making. 13.x sends
androidSafOptionsfordir, andFileUtils.ktbrancheson it:
All five consumers use the answer as a filesystem path. An intermediate draft
of this branch added the argument "to match what the plugin sends" and would
have returned tree URIs on Android — and the two halves of
pickFolderwouldhave disagreed, because the fall-back goes through
FilePicker.getDirectoryPath,whose default is
AndroidOptionsand notFilePickerAndroidOptions, so itsends
nullfor it and still gets a path. The empty map stays, and the file nowsays why it is load-bearing rather than merely equal to the plugin's.
That doc also carried five claims the upgrade falsified, now corrected: the
FilePickerPlugin.javalatch is fixed upstream,file_picker_linux2.0.4 is anXDG portal with no
zenity,FilePickerIOno longer exists, macOS does answerdir, and iOS no longer drops the first request's result handle.saveFileanswers aUriOnly a
file:one names somethingdart:iocan open, so the desktop writeresolves it and returns early otherwise. Android is the platform that does not
give one — its delegate replies with the chosen document's path component
alone, which parses with an empty scheme — and it has already written the bytes
itself.
The test side
FilePicker.platformtoFilePickerPlatform.instance,whose token a subclass inherits, so no mixin and no new dev dependency for
it.
test/picker_fakes.dartabsorbed that and the four files that stillhad their own thirteen-parameter copy — which is what its own doc said this
bump should do.
dropped_pick_test.dartdrives the rejection rather than an empty result,because that is what the platform sends now. Its three cases are still the
three outcomes that differ.
isPickerCallintest/ratchet.dartfollows the API onto the class and isnow a one-line delegation to the helper the
LogServiceratchet uses. Tworatchets read it, and without the change both would have counted zero
pickers and passed while claiming to have checked ten.
folder_pick_test.dart's pinning case drivesFilePickerAndroidwhere itdrove
FilePickerIO, withconst AndroidOptions()— what the facade passes— rather than that class's own default, which would have pinned a call the
app never makes.
android_file_pickeris a dev dependency for that one case.The channel is rebuilt with the default codec: 8.x's implementation used
JSONMethodCodecand the federated ones do not.picker_bytes_guard_test.dartguards two parameters that no longer exist, soit cannot be violated. Its header says so rather than reading as a live rule.
No CHANGELOG entry, because nothing a user can observe changes
Two attempts at one were wrong, which is the argument against inventing a
third. The first claimed an iOS ordering fix that belongs to the photo-library
picker, not the document picker this app uses. The second claimed the save no
longer poisons the remembered folder — but
picker_memory.dart's_remembersis false on Android, so there is no remembered folder there to poison. The
pickers look and behave as they did.
Verified
flutter analyzeclean, 2546 tests,check_format.shclean,check_changelog.shOK.file_picker13.3.3,file_picker_platform_interface4.2.0,android_file_picker2.1.5,file_picker_darwin2.5.1,file_picker_linux2.0.4,windows_file_picker2.0.2 — and for the beforepicture,
file_picker8.3.7's Java.PlatformFile.size,AndroidDeviceInfo.serialNumber, everywin32andwin32_registryAPI. Deployment targets clear the new minimums (darwin wantsiOS 14.0; this app is at 15.0).
What this does not prove
Nothing here ran on a device, and three of the changes are platform
behaviour: the folder pick's arguments, the single-selection pickers and the
save-file scheme check. No job on a pull request compiles Android or iOS — that
is t16 — so the first evidence is the dev build after this merges, and the first
real evidence is a human picking a file.
Also worth knowing
device_info_plus13 does not close the disk-space gap b3 left open: itstill reads
NSFileSystemSizeunconditionally and still ships an emptyNSPrivacyAccessedAPITypes. I said earlier that this bump might close it; itdoes not. The remaining remedy is the guard in our fork of
super_native_extensions, andtest/privacy_manifest_test.dart's pinnedfinding is unchanged.