Add version to pubspec.yaml #16857
Add version to pubspec.yaml #16857
Conversation
|
Great idea! |
There was a problem hiding this comment.
Why not rethrow? If their manifest is invalid and can't be parsed, it seems like we should surface that and halt the build.
There was a problem hiding this comment.
@tvolkert In my first version, I called completer.completeError('test error') with the result that Flutter crashed:
$ flutter build apk --build-name=1.0.3 --build-number=3
Update version in pubspec.yaml... 0,1s
Set version in app/build.gradle... 0,0s
Oops; flutter has exited unexpectedly: "test error".
Sending crash report to Google.
Crash report sent (report ID: da9511e51bac8867)
Crash report written to /Volumes/Archiv/hello/flutter_01.log;
please let us know at https://github.com/flutter/flutter/issues.
How can I break the build without crashing Flutter?
There was a problem hiding this comment.
Note that we want to cause the build to fail, but yes, without the "flutter has exited unexpectedly" flow 😃
There was a problem hiding this comment.
No need to use a Completer any more -- you're async now.
In review comments below, I suggest a new use for a completer, but even then, I think you should await the completer's future in the body of this method, but not return the completer's future.
There was a problem hiding this comment.
Should '' be '.'? Can you add a test that exercises this path?
There was a problem hiding this comment.
The onDone callback won't by awaited by default (its signature returns void). You'll need to create a Completer<void> to signal when it finishes, then await that completer's future below.
There was a problem hiding this comment.
Same goes with onError (but it should probably complete with an error)
There was a problem hiding this comment.
You're async - no need for a completer.
There was a problem hiding this comment.
@tvolkert what is the difference between Future<Null> and Future<void>?
There was a problem hiding this comment.
There was a problem hiding this comment.
The code herein looks very similar to that in gradle.dart -- seems like there's an opportunity to refactor so they share some common code?
|
I'm concerned about having Flutter tooling modify user-edited files of the platform-specific project folders, These parts of a Flutter project are generally intended to be edited by the Flutter user, not Flutter tooling. It is already a problem that Flutter tooling is forced to make assumptions about the contents of these folders (as evidenced by the difficulty of adding Flutter to an existing app). Automatically updating user-edited files is not an easy business (as evidenced by the complexity of the code of this PR) and even when done correctly, it can easily confuse the user, e.g. in relation to the git status of files. For plugin support, Flutter tooling generates files in the platform-specific folders and automatically overwrite those on |
|
For the Android part, I can add a But I have no idea how to do that for iOS. On the other hand, for iOS I use the |
|
On iOS, Flutter tooling already writes |
|
On Android, we communicate information via |
There was a problem hiding this comment.
I guess this is deprecated when I'm done with refactoring suggested by mravn-google
|
You may be interested in this TODO as well: |
ralph-bergmann
force-pushed
the
add_version_to_pubspec
branch
2 times, most recently
from
f061775 to
3940cd0
Compare
|
@mravn-google @tvolkert I refactored my PR, please have a look @Hixie I didn't found an excellent way to solve https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/material/about.dart#L490. But maybe with #16934, we have a developer-friendly way to set the version? |
|
Yeah I wasn't sure how to fix it either :-) |
|
Some PRs ago (#15788) I added the I'm not so sure if it is now the right place because these options are now modifying the I thought about to move them to the What do you think? |
|
@mravn-google @tvolkert friendly reminder :-) |
I would prefer the source of truth, and flow of information from there, to be crystal clear. I see |
|
@mravn-google we use the But at the end, it looks like we have two separate apps and in my opinion, it is one app, a Flutter app. So for me, it looks strange to do it the old/native way instead of creating a new Flutter way. |
|
I agree that we should do it the Flutter way, considering it a single app. A build number set by CI is most likely not something you'd want to check into source control, since CI is often itself triggered by a check in. That would mean the build number cannot be part of The build name is of a different nature, and should be defined by |
|
What about having it in the pubspec, bu can be overridden by a build cli
command? This keeps it simple for smaller projects where you just set it
manually, but scalable for bigger shops who want CI to control the version
and not check it in.
…On Fri, May 18, 2018 at 7:26 AM Mikkel Nygaard Ravn < ***@***.***> wrote:
I agree that we should do it the Flutter way, considering it a single app.
A build number set by CI is most likely not something you'd want to check
into source control, since CI is often itself triggered by a check in.
That would mean the build number cannot be part of pubspec.yaml. It could
be given to flutter build as a CLI argument and relayed to Gradle and
Xcode via the existing channels for transient information:
Generated.xcconfig, local.properties, environment variables.
The build name is of a different nature, and should be defined by version
in pubspec.yaml.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16857 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHAsIbDquN82xZZ4LN5Bdqo27mdIXa4nks5tzq_KgaJpZM4Te6dE>
.
|
|
How about:
|
ralph-bergmann
force-pushed
the
add_version_to_pubspec
branch
from
3940cd0 to
08e9b57
Compare
|
@mravn-google I updated my commit Now the The If the |
ralph-bergmann
force-pushed
the
add_version_to_pubspec
branch
from
08e9b57 to
ac835b5
Compare
There was a problem hiding this comment.
Manifest can't be null here, so need to use ?.
There was a problem hiding this comment.
Manifest can't be null here, so need to use ?.
There was a problem hiding this comment.
You can omit the example since the template value serves as an example
There was a problem hiding this comment.
Maybe rename the flutter build command flag to be --build-version to match this?
There was a problem hiding this comment.
"""
Both the version and the builder number may be overridden in flutter build by
specifying --build-name and --build-number, respectively.
"""
ralph-bergmann
force-pushed
the
add_version_to_pubspec
branch
3 times, most recently
from
937ff73 to
9f5df56
Compare
|
Thank you for spending your time to review this :-) |
|
@tvolkert Can we merge this? |
Uses the `version` property from the `pubspec.yaml` file to set the corresponding fields in the `local.properties` file respectively in the `Generated.xcconfig` file. The `--build-name` and `--build-number` options have change. Now they overtrump the `version` property from the `pubspec.yaml` file. If the `version` property is not set and the `--build-name` and `--build-number` options are not provided, the build command will not change the `local.properties` / `Generated.xcconfig` file.
ralph-bergmann
force-pushed
the
add_version_to_pubspec
branch
from
9f5df56 to
c7fe79c
Compare
|
Yep! |
| } | ||
| } | ||
|
|
||
| testUsingContext('extract build name and number from pubspec.yaml', () async { |
There was a problem hiding this comment.
It looks like these tests aren't hermetic - they're accessing the real file system and spawning processes against the real OS. That makes them subject to significantly more flakiness.
@the4thfloor will you please send a follow-up to move these to be hermetic (using the mock process manager and memory file system, etc.). For example, the tests above use testUsingOsxContext(), which sets up all the hermetic bindings:
flutter/packages/flutter_tools/test/ios/xcodeproj_test.dart
Lines 40 to 44 in bf531ba
|
updating of versionCode via 'flutter build apk --debug --build-number=2' does not stick. I have tested on all channels and when run i witness the change to the local.properties file is in fact undertaken, however a subsequent gradle build step appears to revert to the value to 1 (default). I initially suspected line 15 & 16 in the android/build.gradle file, however when I altered this to a new value, it was not reflected. I've also been checking the generated output.json file in build/app/outputs/apk/debug/ and this is never updated. I am trying to setup CD via fastlane into the App/Play Stores both of which require incremental build version identifiers. Advice appreciated |
|
Hi @gmcdowell, I wrote a Medium article about it, see paragraph Do not hesitate to ask if there are still problems/questions. Ralph |
|
Yeah I spotted that and was following along..
See video
<https://www.dropbox.com/s/hwx532cp7jhxc1i/flutter-versioning-720.mov?dl=0>
of use of build cli tool and the underlying files for apk build affects..
Curious to hear your thoughts.
|
|
I have since discovered that the single source of truth for a flutter's
version is the version in pubspec.yaml file, hence the local.properties
kept being overwritten
My confusion, and others I suspect, is that several articles (including
yours) describe a way of dynamically incrementing the "—build-number"
option either via the flutter build cmd or in fastlane. The issue lies in
the fact that this updated version is NOT reflected back into the
pubspec.yml file, so makes it difficult to automate it would appear.
|
|
there was a discussion about if the change can modify the I will look into the other issue #23811 |
8 participants
Uses the
versionproperty from thepubspec.yamlfile to set the corresponding fields in theapp/build.gradlefile respectively in theInfo.plistfile.The
--build-nameand--build-numberoptions have change. Now they modify thepubspec.yamlfile.The
buildcommand reads at built-time theversionproperty from thepubspec.yamlfile.If the
versionproperty is not set the build command will not change theapp/build.gradle/Info.plistfile.