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 to Dart revision de63e4fa94a1cebf86f0541ac14f25fd0bff73c0 #3442

Conversation

jamesr
Copy link
Contributor

@jamesr jamesr commented Feb 24, 2017

This contains some fixes for Fuchsia and I'd like to keep the version
of Dart used in fuchsia and flutter builds consistent. Should have no
other impactful changes for Flutter.

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. You are most definitely going to have to update the licenses file though.

/me rushes to land his patch.

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

You are most definitely going to have to update the licenses file though.

How do I do that?

@chinmaygarde
Copy link
Member

Instructions are here: https://github.com/flutter/engine/blob/e01d1b39efd5d83de53adeb6413a8dcfd2470dc7/tools/licenses/README. It is quite slow but @jason-simmons is working to make it faster.

@jamesr jamesr force-pushed the dart_revision_de63e4fa94a1cebf86f0541ac14f25fd0bff73c0 branch from ca7dd97 to 0581ed7 Compare February 24, 2017 21:19
@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

30 minutes of chugging later, let's see how this one goes

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

OK never mind, time to regenerate :(

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

Hmm, apparently the licenses golden file is platform-specific as well, since it contains the contents of the android NDK which is different on different host platforms:

====================================================================================================
LIBRARY: ndk
-ORIGIN: ../../../third_party/android_tools/ndk/shader-tools/linux-x86_64/NOTICE
+ORIGIN: ../../../third_party/android_tools/ndk/shader-tools/darwin-x86_64/NOTICE
TYPE: LicenseType.apache

... so time to regenerate from a linux box.

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

After rebasing on linux I'm unable to regenerate the licenses.golden file:

$ time dart --checked lib/main.dart ../../.. > ../../travis/licenses.golden
Finding files...
Collecting licenses...
31569 of 31570 ██████████ 100% (0 missing licenses) ../../../third_party/zlib/zutil.h error searching for copyright in: ../../../tagsson
'file:///ssd/flutter_engine/src/flutter/tools/licenses/lib/main.dart': Failed assertion: line 2161 pos 12: 'false' is not true.
#0 _AssertionError._throwNew (dart:core-patch/errors_patch.dart:24)
#1 _AssertionError._checkAssertion (dart:core-patch/errors_patch.dart:31)
#2 RepositoryRoot.libraryName (file:///ssd/flutter_engine/src/flutter/tools/licenses/lib/main.dart:2161:12)
#3 RepositoryFile.libraryName (file:///ssd/flutter_engine/src/flutter/tools/licenses/lib/main.dart:39:36)
#4 RepositorySourceFile.licenses. (file:///ssd/flutter_engine/src/flutter/tools/licenses/lib/main.dart:100:74)
#5 List.forEach (dart:core-patch/growable_array.dart:258)
#6 RepositorySourceFile.licenses (file:///ssd/flutter_engine/src/flutter/tools/licenses/lib/main.dart:100:15)
#7 RepositoryDirectory.getLicenses (file:///ssd/flutter_engine/src/flutter/tools/licenses/lib/main.dart:1210:41)
#8 main (file:///ssd/flutter_engine/src/flutter/tools/licenses/lib/main.dart:2251:57)
#9 _startIsolate. (dart:isolate-patch/isolate_patch.dart:259)
#10 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:148)

 31570 of 31570 ██████████ 100% (1 missing licenses)  Done.    

real 21m19.878s
user 23m16.602s
sys 0m16.881s

Note the time there - 21 minutes :(

The zlib header I have matches https://github.com/flutter/buildroot/blob/master/third_party/zlib/zutil.h

/cc @jason-simmons @Hixie

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

Ah, maybe it failed because I have a ctags file locally. Will remove and try again and find out in 20 minutes if that was it. This is a highly frustrating process.

@jamesr jamesr force-pushed the dart_revision_de63e4fa94a1cebf86f0541ac14f25fd0bff73c0 branch from 0581ed7 to dfe996b Compare February 24, 2017 23:28
@abarth
Copy link
Contributor

abarth commented Feb 24, 2017

This is a highly frustrating process.

Yes.

Hopefully this patch will make it 33x faster:

#3437

/cc @Hixie

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

#3437 won't help at all for this, or future Dart rolls, since this changes things outside the flutter/ tree.

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

Travis says we aren't going to space today:

Total license count: 695

  • 31569 of 31569 ██████████ 100% (0 missing licenses)  Done.    
    
  • 31567 of 31567 ██████████ 100% (0 missing licenses)  Done.  
    

@jamesr
Copy link
Contributor Author

jamesr commented Feb 24, 2017

Ah, it found some .pyc files in my tree. Time to go again..

@abarth
Copy link
Contributor

abarth commented Feb 25, 2017

since this changes things outside the flutter/ tree

It should still skip analyzing third_party, right? I presume that's where most of the time goes. But yes. This area is painful currently.

@jamesr jamesr force-pushed the dart_revision_de63e4fa94a1cebf86f0541ac14f25fd0bff73c0 branch from dfe996b to 796457d Compare February 25, 2017 00:01
@jamesr
Copy link
Contributor Author

jamesr commented Feb 25, 2017

Done.    License script got different results than expected.
Please rerun the licenses script locally to verify that it is
correctly catching any new licenses for anything you may have
changed, and then update this file:
  flutter/sky/packages/sky_engine/LICENSE
For more information, see the script in:
  https://github.com/flutter/engine/tree/master/tools/licenses
--- flutter/travis/licenses.golden	2017-02-25 00:02:26.731273425 +0000
+++ out/license-script-output	2017-02-25 00:28:17.107273425 +0000
@@ -72132,7 +72132,7 @@
    appreciated but is not required.
 2. Altered source versions must be plainly marked as such, and must not be
    misrepresented as being the original software.
 3. This notice may not be removed or altered from any source distribution.
 ====================================================================================================
 Total license count: 695
-     31567 of 31567 ██████████ 100% (0 missing licenses)  Done.
+     31567 of 31567 ██████████ 100% (0 missing licenses)  Done.    

Well then...

This contains some fixes for Fuchsia and I'd like to keep the version
of Dart used in fuchsia and flutter builds consistent. Should have no
other impactful changes for Flutter.
@jamesr jamesr force-pushed the dart_revision_de63e4fa94a1cebf86f0541ac14f25fd0bff73c0 branch from 796457d to 559c875 Compare February 25, 2017 00:52
@jamesr jamesr merged commit d8a8f10 into flutter:master Feb 25, 2017
@jamesr jamesr deleted the dart_revision_de63e4fa94a1cebf86f0541ac14f25fd0bff73c0 branch February 25, 2017 01:19
goderbauer pushed a commit to goderbauer/engine that referenced this pull request Mar 9, 2017
This contains some fixes for Fuchsia and I'd like to keep the version
of Dart used in fuchsia and flutter builds consistent. Should have no
other impactful changes for Flutter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants