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

Fixes ac for multiple publishers (uplift to 1.10.x) #5736

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Jun 4, 2020

Uplift of #5728
Resolves brave/brave-browser#10058

Approved, please ensure that before merging:

  • You have checked CI and the builds, lint, and tests all pass or are not related to your PR.
  • You have tested your change on Nightly.
  • The PR milestones match the branch they are landing to.

After you merge:

  • The associated issue milestone is set to the smallest version that the changes is landed on.

@NejcZdovc NejcZdovc added this to the 1.10.x - Beta milestone Jun 4, 2020
@NejcZdovc NejcZdovc requested a review from a team June 4, 2020 06:43
@NejcZdovc NejcZdovc self-assigned this Jun 4, 2020
@NejcZdovc NejcZdovc force-pushed the multiple-publishers-ac-1.10 branch from 7ea1d0c to 2bcd19b Compare June 4, 2020 07:22
@kjozwiak
Copy link
Member

kjozwiak commented Jun 9, 2020

Looks like test-browser failed on Linux due to the following https://ci.brave.com/job/pr-brave-browser-multiple-publishers-ac-1.10/2/execution/node/417/log/

04:17:03  1 test failed:
04:17:03      BraveRewardsBrowserTest.CheckIfReconcileWasResetEmpty (../../brave/components/brave_rewards/browser/rewards_service_browsertest.cc:2941)

Above is a known issue and has been resolved via brave/brave-browser#10032. Doesn't block uplift.

@NejcZdovc @bsclifton looks like both test-unit and test-browser failed on macOS due to speedreader as per https://ci.brave.com/job/pr-brave-browser-multiple-publishers-ac-1.10/2/execution/node/528/log/ & https://ci.brave.com/job/pr-brave-browser-multiple-publishers-ac-1.10/2/execution/node/495/log/. Example:

04:39:53  FAILED: alwaysrununittests/test_install_name_speedreader 
04:39:53  python ../../build/gn_run_binary.py /usr/bin/install_name_tool -change @loader_path/../../Libraries/libspeedreader_ffi.dylib @executable_path/gen/speedreader/out/x86_64-apple-darwin/release/libspeedreader_ffi.dylib brave_unit_tests
04:39:53  error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool: changing install names or rpaths can't be redone for: brave_unit_tests (for architecture x86_64) because larger updated load commands do not fit (the program must be relinked, and you may need to use -headerpad or -headerpad_max_install_names)
04:39:53  /usr/bin/install_name_tool failed with exit code 1
04:39:53  [3/6] STAMP obj/brave/components/speedreader/rust/ffi/test_install_name_speedreader_browser_tests.stamp

However, assuming this code doesn't touch any of the speedreader code. Is the above failure known?

@NejcZdovc
Copy link
Contributor Author

@kjozwiak yup code doesn't touch speedreader, so completely unrelated

Copy link
Member

@kjozwiak kjozwiak left a comment

Choose a reason for hiding this comment

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

Uplift into 1.10.x approved after deliberating with @brave/uplift-approvers. QA also verified the PR on Nightly as per #5728 (comment).

@kjozwiak kjozwiak merged commit 5dd6be3 into 1.10.x Jun 9, 2020
@kjozwiak kjozwiak deleted the multiple-publishers-ac-1.10 branch June 9, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants