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.

Sign up for GitHub

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

Jump to bottom

Simplify changes to upstream's //chrome/installer when possible #14364

Open
mariospr opened this issue Feb 25, 2021 · 2 comments
Open

Simplify changes to upstream's //chrome/installer when possible #14364

mariospr opened this issue Feb 25, 2021 · 2 comments
Labels
dev-concern OS/Desktop OS/Windows QA/Yes release-notes/exclude

Comments

@mariospr
Copy link
Contributor

Description

As pointed out by @simonhong in brave/brave-core#8063 (comment), it seems that we have patches and chromium_src overrides affecting files under //chrome/installer that are supporting the official and non-official builds when, in reality, the installer builds are not supported for non-official builds.

This means that we should be able to simplify those patches & chromium_src overrides further, perhaps even leading to removing some patches, so this issue is to track that effort.

Brave version (brave://version info)

Brave 1.23.1 (Chromium 89.0.4389.58)
@mariospr mariospr added this to the 1.23.x - Nightly milestone Feb 25, 2021
@mariospr mariospr self-assigned this Feb 25, 2021
This was referenced Feb 25, 2021
Merged
Closed
mariospr added a commit to brave/brave-core that referenced this issue Feb 25, 2021
@mariospr
Fix windows build due to invalid assignment to a constant
2d9f74f 
While landing PR #8063 [1], we didn't realize that the assignment in
the chromium_src override was wrong because the CI bots didn't complain,
since the installer is not supported in non official builds, and
because I normally build on Linux only.

Then I filed issue 14365 [2] to cleanup all those non-official code
paths from changes to //chrome/installer/** files and, as soon as I
started building on my Windows machine with official_build = false
I hit a build error because of this assignment.

Therefore, while the long term fix will come as part of fixing that
other issue, it would be great to get non-official Windows building
again.

[1] https://github.com/brave/brave-core/pull/8063/files
[2] brave/brave-browser#14364

Resolves brave/brave-browser#14365
@mariospr mariospr mentioned this issue Feb 25, 2021
Merged
24 tasks
@kjozwiak
Copy link
Member

kjozwiak commented Mar 30, 2021 •
edited
Loading

@mariospr removing this from the milestone as it's not a blocker and seems like it doesn't have a PR that's in progress. Usually the only time an issue should be opened within a milestone is when it's a release blocking and needs to go out in that specific milestone.

Please re-add if this is supposed to land in 1.23.x or should have been closed. But from the looks of things, it's still pending triage.

@kjozwiak kjozwiak removed this from the 1.23.x - Beta milestone Mar 30, 2021
@mariospr
Copy link
Contributor Author

mariospr commented Apr 5, 2021

@kjozwiak thanks for doing that. I have a WIP PR locally but didn't advance it well enough yet to push it into a PR due to other more pressing task. Will update this PR once I have something more mature to propose.

@mariospr mariospr removed their assignment Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
dev-concern OS/Desktop OS/Windows QA/Yes release-notes/exclude
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mariospr @kjozwiak @rebron