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

[Bromite Builder] [Prepare] Use ungoogled-chromium for domain substitutions #9

Merged
merged 2 commits into from
May 20, 2020

Conversation

nikolowry
Copy link
Owner

@csagan5 the Automated-domain-substitutions.patch patch file has the potential of being a source of constant failures -- it's a lot to manage, especially when just one hunk of its 4mb contents could cause rejections.

So this branch was created to explore using ungoogled-chromium's domain_substitution.py to dynamically apply the substitutions before builds.

Do you use a custom domain_regex.list or domain_substitution.list? If you do, is there anyway you'd be able to start checking those in?

I might revert this feature (or turn this into a user option), if the next few releases the patch applies cleanly.

If you don't feel like maintaining such a patch file, there's always other routes you could take in the Bromite repo:

  1. Add a note to the README explaining how to use ungoogled-chromium's domain-substitution.py when building locally
  2. Add ungoogled-chromium as a sparse and shallow submodule and only add utils/domain-substitution.py, domain_regex.list and domain_substitution.list

Thanks in advance for any insights!

…utions

Note: Python 3 is now a required dependency
@nikolowry
Copy link
Owner Author

Automated-domain-substitution.patch has two failing hunks on latest release 81.0.4103.53:

patching file chrome/browser/ui/android/strings/android_chrome_strings.grd
Hunk #8 succeeded at 1483 (offset 3 lines).
Hunk #9 succeeded at 1497 (offset 3 lines).
Hunk #10 FAILED at 1547.
Hunk #11 succeeded at 1590 (offset 12 lines).
Hunk #12 succeeded at 1738 (offset 12 lines).
Hunk #13 succeeded at 1783 (offset 12 lines).
Hunk #14 succeeded at 1978 (offset 12 lines).
Hunk #15 succeeded at 2240 (offset 12 lines).
Hunk #16 succeeded at 2249 (offset 12 lines).
Hunk #17 succeeded at 3203 (offset 15 lines).
Hunk #18 succeeded at 3215 (offset 15 lines).
Hunk #19 succeeded at 3415 (offset 15 lines).
Hunk #20 succeeded at 3464 (offset 15 lines).
Hunk #21 succeeded at 3768 (offset 15 lines).
patching file components/ntp_tiles/resources/default_popular_sites.json
Hunk #1 FAILED at 1.

@csagan5
Copy link
Contributor

csagan5 commented May 16, 2020

one hunk of its 4mb

It used to be much bigger (~92MB) because it included tests, I have excluded them since then.

Do you use a custom domain_regex.list or domain_substitution.list? If you do, is there anyway you'd be able to start checking those in?

Yes, I even started to upstream them in https://github.com/wchen342/ungoogled-chromium-android

I could release them but I also modified the script, see ungoogled-software/ungoogled-chromium#1026

So the patch is the most convenient way for now.

The rejections and patch issues are there simply because I have uncommitted changes due to text replacements (Chromium -> Bromite); I have now started creating the patch (see bromite/bromite@4c4dd71) before applying the text replacements so there should not be anymore issues with this patch as it will be equal to any other.

@nikolowry
Copy link
Owner Author

Thanks for the input @csagan5! I'll leave this open and revisit after a few more releases.

…allback

Always attempt to use `Automated-domain-substituion.patch` first, but
if it fails automatically fallback to Ungoogled's utility
@nikolowry
Copy link
Owner Author

Merging this PR after revising the initial feature.

We'll only use Ungoogled's domain substition utility as a fallback when Automated-domain-substitutions.patch can't be applied. So rather than prefer the patch or the utility, we'll make use of both -- this feels like the safest route to take.

@nikolowry nikolowry merged commit ba20145 into master May 20, 2020
@csagan5
Copy link
Contributor

csagan5 commented May 20, 2020

@nikolowry some bugs can be introduced when applying the default lists because not all files are skipped.

Another common reason for failure to apply has been the Chromium version, I have thought to put that in some file perhaps to avoid using tags alone.

@nikolowry nikolowry deleted the feature/ungoogled-domain-substitution branch May 20, 2020 16:43
@nikolowry
Copy link
Owner Author

@csagan5 understood on bug possibilities -- I'll keep an eye on it and add a way to opt out of the fallback.

Love the idea of a version.txt, was actually going to open an issue for you to consider it. If implemented, it would be super easy for me to do test builds prior to official releases. As of now, I run ./bromite-builder --upstream to do builds of your master's HEAD, but the Chromium revision defaults to latest Bromite release unless it is explicitly set with --revision=<revision>.

@csagan5
Copy link
Contributor

csagan5 commented Jun 1, 2020

FYI I have added the version file a couple weeks ago as build/RELEASE.

@nikolowry
Copy link
Owner Author

@csagan5 dope! I'll be looking in to how I can integrate that into fetch-sync this month

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants