Started identifying fuchsia sdk rom files as binary files in license checker #41900

gaaclarke · 2023-05-10T16:48:06Z

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

gaaclarke · 2023-05-10T17:39:55Z

This solves the issue but now we are running into an infra issue:

Finding files...
Detected changes to license tool. Forcing license collection for all components.
Collecting licenses for flutter

Collecting licenses for fuchsia

Collecting licenses for gpu

Collecting licenses for third_party

failure: ProcessException: No such file or directory
  Command: gclient revinfo
#0      _ProcessImpl._runAndWait (dart:io-patch/process_patch.dart:487:7)
#1      _runNonInteractiveProcessSync (dart:io-patch/process_patch.dart:632:18)
#2      Process.runSync (dart:io-patch/process_patch.dart:68:12)
#3      _RepositoryRootCertificatesDirectory.officialSourceLocation (package:licenses/main.dart:1604:56)
#4      MozillaLicense.toStringBody (package:licenses/licenses.dart:1314:93)
#5      groupLicenses (package:licenses/licenses.dart:219:44)
#6      _collectLicensesForComponent (package:licenses/main.dart:1951:41)
<asynchronous suspension>
#7      main (package:licenses/main.dart:2094:9)
<asynchronous suspension>

aam · 2023-05-10T20:09:02Z

tools/licenses/lib/filesystem.dart

    case '.icc': return FileType.binary; // Color profile
    case '.swp': return FileType.binary; // Vim swap file
    case '.bfbs': return FileType.binary; // Flatbuffers Binary Schema
+    case '.rom': return FileType.binary;


.rom-files that are part of fuchsia/sdk/linux/tools should be already skipped by https://github.com/flutter/engine/blob/main/tools/licenses/lib/paths.dart#L40.
Were you able to confirm where other .rom-files are coming?

Yea, it's in the linked issue: ../../../fuchsia/sdk/core.tar.gz!core.tar!/tools/arm64/aemu_internal/lib/pc-bios/efi-e1000.rom.

Were you able to confirm where core.tar.gz came from? I don't think it's part of fuchsia sdk.

Nope, I didn't look deeply into it since this is a blocker and it's there somehow, and in no case do we want to do license checks on rom files. I assume its presence is justified somewhere else since the engine code isn't doing it.

I would be wary of this being another symptom of some issue with newly introduced license builder. Similar to how 'gclient' was missing, for example.

(I just verified that srujans PR still needs this fix)

actually, every PR needs this

if we're not expecting the core.tar.gz file we should figure out where it is coming from.

(especially a .tar.gz file, which is probably adding more time to the already slow license script process)

I filed an issue: flutter/flutter#126475

gaaclarke · 2023-05-10T20:34:25Z

CI seems hung, i'll push an empty commit to retrigger everything.

gaaclarke · 2023-05-10T20:42:10Z

~~The engine tree is now closed on this failure~~

Sorry, this is a different license checker error that was related to godofredos revert.

…checker.

flutter-dashboard · 2023-05-10T23:46:47Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

gaaclarke · 2023-05-10T23:55:50Z

This may not be necessary. Godofredo reverted the engine_v2 license checker that was displaying this error in #41903, so no one is blocked on this.

We have an issue for the residual files that seem to be tripping up the license checker and godofredo has an idea how to fix it.

chinmaygarde · 2023-05-11T20:16:08Z

From Triage: This was an issue with the bot not clearing artifacts from previous runs and should no longer be necessary. Closing.

This was referenced May 10, 2023

license checker failing on missing gclient revinfo flutter/flutter#126447

Closed

License checker is tripping up on fuchsia artifacts flutter/flutter#126441

Closed

gaaclarke marked this pull request as ready for review May 10, 2023 19:58

gaaclarke requested a review from Hixie May 10, 2023 19:58

gaaclarke force-pushed the ignore-fuchsia-artifacts-licenses branch from d1c0d30 to b25562a Compare May 10, 2023 20:00

aam reviewed May 10, 2023

View reviewed changes

gaaclarke force-pushed the ignore-fuchsia-artifacts-licenses branch from b25562a to 12bcb6e Compare May 10, 2023 20:35

gaaclarke requested a review from zanderso May 10, 2023 20:42

gaaclarke added 2 commits May 10, 2023 14:49

Started identifying fuchsia sdk rom files as binary files in license …

f1ea39d

…checker.

switchs to extension test

516b9ad

gaaclarke force-pushed the ignore-fuchsia-artifacts-licenses branch from 12bcb6e to 516b9ad Compare May 10, 2023 21:49

updated signature

5e47198

zanderso approved these changes May 10, 2023

View reviewed changes

gaaclarke mentioned this pull request May 10, 2023

Mysterious fuchsia/sdk/core.tar.gz showing up in Linux linux_license runs flutter/flutter#126475

Closed

gaaclarke changed the title ~~Started identifying fuchsia sdk rom files as binary files in license checker~~ Started identifying fuchsia sdk rom files as binary files in Linux linux_license May 10, 2023

flutter-dashboard bot added the needs tests label May 10, 2023

gaaclarke changed the title ~~Started identifying fuchsia sdk rom files as binary files in Linux linux_license~~ Started identifying fuchsia sdk rom files as binary files in license checker May 10, 2023

chinmaygarde closed this May 11, 2023

Started identifying fuchsia sdk rom files as binary files in license checker #41900

Started identifying fuchsia sdk rom files as binary files in license checker #41900

Uh oh!

Conversation

gaaclarke commented May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gaaclarke commented May 10, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aam May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented May 10, 2023

Uh oh!

gaaclarke commented May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented May 10, 2023

Uh oh!

gaaclarke commented May 10, 2023

Uh oh!

chinmaygarde commented May 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gaaclarke commented May 10, 2023 •

edited

Loading

aam May 10, 2023 •

edited

Loading

gaaclarke commented May 10, 2023 •

edited

Loading