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

update win32 #1281

Merged
merged 1 commit into from
May 25, 2023
Merged

update win32 #1281

merged 1 commit into from
May 25, 2023

Conversation

frg2089
Copy link

@frg2089 frg2089 commented May 13, 2023

it may worked on win32: '>=3.0.0 <6.0.0'.
close #1280

@miguelpruivo miguelpruivo requested a review from philenius May 13, 2023 16:02
@philenius
Copy link
Collaborator

Hi @frg2089, I understand your request to update win32 to the latest major version 5.0.0. However, generally speaking, it is highly-dangerous to specify the version of a dependency in such a loose manner, e.g. win32: '>=3.0.0 <6.0.0', such that various different major versions (v3, v4, or v5) could be installed. On your laptop, dart pub might install v5, whereas on mine, it could install v4. SemVer, semantic versioning, specifies that the major version should be incremented

"when you make incompatible API changes"

see https://semver.org/.

I cannot merge your changes to pubspec.yaml since file_picker directly calls the API of win32 and win32 introduced breaking changes to its public API with v5. Additionally, the README of win32 states the following:

Backwards compatibility

The library version models semver, but you cannot assume a strict guarantee of no breaking changes between minor versions. That guarantee is not possible to make, for several reasons:

  • The package is based on metadata published by Microsoft, which is generated by scraping Win32 SDK header files. As the quality of the scraper improves, there may be minor changes to some fields (for example, an unsigned integer may become a signed integer).
  • Adding new APIs may itself cause a breaking change. For example, if you declare a missing Windows constant in your own code that is then added, Dart will complain about the duplicate definition.
    If this causes you concern, our recommendation is to pin to a specific version of Win32. But the best approach is simply to test regularly with the latest version of this package, and continue to move your minimum forward.

My suggestion: we pin the version of win32 to ^5.0.2 and release a new patch version of file_picker.

@philenius
Copy link
Collaborator

@miguelpruivo: last time when we bumped win32 from v3 to v4, we released a new patch version of file_picker (see #1266 (comment)). For consistency, now as we updated win32 from v4 to v5, I bumped the patch version of file_picker again.

The only difference: this time, I had to upgrade my Flutter installation from from 3.7.12 to 3.10.1 because initially I got this error when I updated win32 to v5:

Because file_picker depends on win32 >=5.0.0 which requires SDK version >=3.0.0 <4.0.0, version solving failed

I don't fully comprehend the implications of this Flutter upgrade. Originally, I thought that I would have to change the line in pubspec.yaml that says

environment:
  sdk: ">=2.17.0 <3.0.0"

However, it turned out that upgrading Flutter to the latest version was already sufficient.


If you're fine with bumping the patch version of file_picker, then this PR is ready to be merged.

@philenius philenius merged commit a8346ee into miguelpruivo:master May 25, 2023
@stuartmorgan
Copy link

stuartmorgan commented May 29, 2023

However, generally speaking, it is highly-dangerous to specify the version of a dependency in such a loose manner, e.g. win32: '>=3.0.0 <6.0.0', such that various different major versions (v3, v4, or v5) could be installed.

FYI, this is not correct. There is nothing dangerous about expressing what is actually required of a dependency.

It would be dangerous to do that if those versions were untested. If, however, something was written against version X.0.0 of a package, and compiles and works without changes with version X+1.0.0, then a range spanning two major versions accurately expresses the version requirements.

On your laptop, dart pub might install v5, whereas on mine, it could install v4.

If the package works correctly, that's not an issue. It is in fact a feature, since it increases compatibility with other packages (e.g., those that haven't been updated to the latest major version of the dependency).

SemVer, semantic versioning, specifies that the major version should be incremented

"when you make incompatible API changes"

see https://semver.org/.

The existence of breaking changes somewhere in an API surface does not mean that all usage of any part of that API surface will be broken.

A simple example is a major version change that does nothing but remove deprecated API. Clients that have already stopped using the API when it was originally deprecated are completely unaffected by the change.

@philenius
Copy link
Collaborator

Thanks @stuartmorgan , I learned something new :)

From your pubspec.yaml in path_provider_windows, I can see that in previous versions of file_picker we could have put the version constraint like this: win32: ">=2.1.0 <5.0.0". However, with v5 of win32 there's an actual breaking change as we had to change the import statement from

import 'package:win32/winrt.dart';

to

import 'package:win32/win32.dart';

Therefore, I thought that we could not have put a lose version constraint such as win32: '>=3.0.0 <6.0.0'.

@gslender
Copy link

gslender commented Jun 4, 2023

@stuartmorgan if the comment from @philenius is correct, are you able to get package path_provider updated as the change made in flutter/flutter#127289 has blocked the latest version of this package from being used

@stuartmorgan
Copy link

as the change made in flutter/flutter#127289 has blocked the latest version of this package from being used

I'm not following your comment, sorry. No change has been made in that issue, and no change to path_provider has blocked anything. The change that prevents you from using the the latest version of this package with path_provider (or anything that hasn't been updated to use the absolute latest version of win32) is this PR.

@gslender
Copy link

gslender commented Jun 5, 2023

path_provider has a fixed dependency on win32 that prohibits later releases of win32, which is now blocking later releases of other packages that are depending on later releases of win32 fixes.

Seems odd that path_provider can't use any version greater than v5.0.x which would have allowed file_picker and others to be updated.

@gslender
Copy link

gslender commented Jun 5, 2023

Sorry. I just now found the PR and as you didn't respond to the comment earlier I incorrectly assumed no action was taken

@stuartmorgan
Copy link

path_provider has a fixed dependency on win32 that prohibits later releases of win32, which is now blocking later releases of other packages that are depending on later releases of win32 fixes.

Please re-read my comment. I understand the dependency conflict. What I don't understand, as I said there, is your attribution of that conflict to a change in path_provider.

Seems odd that path_provider can't use any version greater than v5.0.x which would have allowed file_picker and others to be updated.

I assume you mean greater than 4.x. I'm not sure what you find odd about path_provider needing to be updated, but path_provider is off topic here; if you have an issue with path_provider, please file an issue in its issue tracker.

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.

Update win32 please
4 participants