Download external patches

Applying patch series for comm repository
Applying patch 01-branding.patch ...
Applying patch 02-branding.patch ...
Applying patch 03-branding.patch ...
Applying patch 03-branding-mac.patch ...
Applying patch 04-branding.patch ...
Applying patch 05-branding.patch ...
../patches/05-branding.patch:148: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying patch 06-branding.patch ...
Applying patch 07-branding.patch ...
Applying patch 99-misc-release-set-version.patch ...
Applying patch 01-misc-account-provisioner.patch ...
Applying patch 02-misc-user-agent-string.patch ...
Applying patch 03-misc-update-check.patch ...
Applying patch 05-misc-no-multi-lingual.patch ...
Applying patch 01-feature-activity-manager-in-tab.patch ...
Applying patch 02-feature-Windows-systray-tooltip.patch ...
Applying patch 03-feature-biff-server.patch ...
Applying patch 04-feature-multiline-tree.patch ...
Applying patch 04-feature-multiline-tree-allow-grouping.patch ...
Applying patch 04-feature-multiline-tree-allow-threading.patch ...
Applying patch 04-feature-multiline-tree-dummy-rows-css.patch ...
Applying patch 05-feature-default-buttons.patch ...
Applying patch 07-feature-open-news-article-by-ID.patch ...
Applying patch 08-feature-startup-folder.patch ...
Applying patch 09-feature-attach-recent.patch ...
Applying patch 683809-QFB-untagged.patch ...
Applying patch 188988-Gloda-search-encrypted.patch ...
Applying patch 1729221-ignore_missing_mdc.patch ...
Applying patch 1562737-body-search-encrypted.patch ...
Applying patch 1680425-dont-overwrite-subject.patch ...
Applying patch 350825-commas-detached-attachment.patch ...
Applying patch 1740956-detached-attachment-location.patch ...
Applying patch 1739609-more-charset-issues.patch ...
Applying patch 1737245-forward-charset-detect.patch ...
Applying patch 1746749-get-feed-as-html.patch ...
Applying patch 697031-make-thread-pane-address-display-configurable.patch ...
Applying patch 799040-fix-unread-in-ignored-thread.patch ...
Applying patch 1751160-news-folder-notify-alt.patch ...
Applying patch 1751160-news-folder-notify.patch ...
Applying patch 589008-move-multiple-imap-folders.patch ...
Applying patch 1744709-save-double-click-download.patch ...
Applying patch 532712-insert-NBSP.patch ...
Applying patch 1760365-image-copy-no-filename.patch ...
Applying patch 1762043-fix-startup-incomplete.patch ...
Applying patch 1764842-threaded-view-reversal.patch ...
Applying patch 10-feature-more-chars-insert-char-symb.patch ...
Applying patch 1766578-fix-null-status-text.patch ...
Applying patch 11-feature-automatic-plain-or-html.patch ...
Applying patch 1676825-save-composition.patch ...
Applying patch 297852-grouping-search-term.patch ...
../patches/297852-grouping-search-term.patch:949: trailing whitespace.

warning: 1 line adds whitespace errors.
Applying patch backout-bug-1775093-temp-fix-for-missing-string.patch ...
Applying patch 1775376-fix-dict-edit-draft.patch ...
Applying patch 1783517-password-dialogue-cancel.patch ...
Applying patch 1777276-fix-startup-console-error.patch ...
Applying patch 1780963-gloda-undefined-display.patch ...
Applying patch 1782418-fix-create-contact-from-compose.patch ...
error: patch failed: mail/components/compose/content/messengercompose.xhtml:889
error: mail/components/compose/content/messengercompose.xhtml: patch does not apply

@Betterbird I am trying to apply this patch series from your repo to the Thunderbird source code for version 102.2.1 by going through the series file line by line and using git apply to apply the respective patch. Shouldn't that work?

flathubbot commented 2 years ago

Build 108412 was cancelled

flathubbot commented 2 years ago

Build 108408 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/106068/eu.betterbird.Betterbird.flatpakref

Betterbird commented 2 years ago

I've fixed the empty line and trailing whitespace issue. As for the apply failure: We use hg qpush and for this particular patch with is take directly from Mozilla as per 1782418-fix-create-contact-from-compose.patch # https://hg.mozilla.org/comm-central/rev/1185521d6cb5 we get an apply warning but no error: patching file mail/components/compose/content/messengercompose.xhtml Hunk #1 succeeded at 896 with fuzz 1 (offset 6 lines).

Is that a problem? If so, we'd have to rebase the patch and store it ourselves.

Betterbird commented 2 years ago

I've fixed the fuzz issue, too, please let me know whether you require this in the future. Apparently hg qpush is more tolerant than git apply: https://github.com/Betterbird/thunderbird-patches/commit/80e46b75f80cf51c6195b7bb899857462f2c592e

Of course, if you're using this revision, you're not building 102.2.1 but some other "latest build".

Also note that the patches only apply when pushed onto the correct base revisions as noted here: https://github.com/Betterbird/thunderbird-patches/blob/main/102/102.sh

flathubbot commented 2 years ago

Started test build 108498

flathubbot commented 2 years ago

Build 108498 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/106156/eu.betterbird.Betterbird.flatpakref

Betterbird commented 2 years ago

Is there anywhere we can check the log? Like pulling the Mozilla repos, applying the patches, doing the build an packaging?

mfschumann commented 2 years ago

Yes. You get there by clicking on the link "Build xyz successful" posted by flathubbot above and then expanding the Build step twice.

It seems that your fix worked and 1782418-fix-create-contact-from-compose.patch was applied successfully now.

mfschumann commented 2 years ago

Also note that the patches only apply when pushed onto the correct base revisions as noted here: https://github.com/Betterbird/thunderbird-patches/blob/main/102/102.sh

Is it safe to assume that e.g. for Betterbird 102.2.1-bb15 you are taking the source code of the Thunderbird 102.2.1 release? I currently do not checkout the Thunderbird repo at the commit you specify, but I just download the source archive for the release.

mfschumann commented 2 years ago

I've fixed the fuzz issue, too, please let me know whether you require this in the future.

I will check if I can convince git apply to apply the original patch and let you know about the results.

Betterbird commented 2 years ago

Is it safe to assume that e.g. for Betterbird 102.2.1-bb15 you are taking the source code of the Thunderbird 102.2.1 release? I currently do not checkout the Thunderbird repo at the commit you specify, but I just download the source archive for the release.

Hmm, taking the source code (tarball?) is of course easier then pulling the repo and using HG. Good lateral thinking ;-) - Please remind me where you get the source from and how the revision relates to the source. I guess TB publish it with each release or each release build. Mostly they publish build1, but at times they publish build 2 or build 3. We're always based on a TB build, that is, the revision found in 102.sh corresponds to a real build.

However, we're not necessarily based on the latest published TB build. For example, for TB 102.2.0 they shipped build 3 where they had fixed a bad issue from build 2 via a backout. We shipped BB 102.2.0 based on build 2 and fixed the issue differently.

Are you familiar with https://treeherder.mozilla.org/jobs?repo=comm-esr102? You can see all the action there for TB. That's where we look to pick a changeset for our builds.

EDIT: Looks like you get the source from here: http://ftp.mozilla.org/pub/thunderbird/candidates/102.2.1-candidates/build1/source/ So you can get source for various builds. So how would you like the build recorded? Or can you work it out from the changeset? Or should we add it to eu.betterbird.Betterbird.appdata.xml file? How? Like so?

  <releases>
    <release version="102.2.1-bb15" date="2022-09-01" base="build1"/>
  </releases>

Betterbird commented 2 years ago

Why do you go through pain of excluding 08-branding-m-c.patch which adds some Windows-specific stuff? That doesn't execute on Linux, so no need to exclude it. We don't exclude it in our Linux builds.

Betterbird commented 2 years ago

General remark: In case you need a quick reply, please contact us via e-mail. Afk we don't have access to GitHub.

EDIT: One more thing. TB is building 102.2.2 now, we'll do it in due course, maybe tomorrow, you can use your build for real.

mfschumann commented 2 years ago

Ah, I wasn't aware there could be multiple builds of the same release with different sources.

I think the correct way to communicate which build you used is via the .appdata.xml. You can add an artifact to the release section that points to the correct source archive. I will then use that exact archive.

Like so:

<releases>
  <release version="102.2.1-bb15" date="2022-09-01">
    <artifacts>
      <artifact type="source">
        <location>http://ftp.mozilla.org/pub/thunderbird/candidates/102.2.1-candidates/build1/source/thunderbird-102.2.1.source.tar.xz</location>
        <checksum type="sha256">....</checksum>
      </artifact>
    </artifacts>
  </release>
</releases>

mfschumann commented 2 years ago

Why do you go through pain of excluding 08-branding-m-c.patch which adds some Windows-specific stuff? That doesn't execute on Linux, so no need to exclude it. We don't exclude it in our Linux builds.

That patch lead to errors in an early version of my build script, but it now works (with warnings, though). I probably don't need to comment it out anymore.

Betterbird commented 2 years ago

That patch lead to errors in an early version of my build script, but it now works (with warnings, though). I probably don't need to comment it out anymore.

Which warning? Maybe about mixed use of LR and CRLF. The Windows file it creates needs to have CRLF. I can't see anything else wrong with it.

I can add the artifact section, but where did you get the source from in the past? BTW, 102.2.2 will happen today or tomorrow.

Betterbird commented 2 years ago

Umm, where do you want me to get the checksum from? Please take a look at: http://ftp.mozilla.org/pub/thunderbird/candidates/102.2.1-candidates/build1/source/ They don't publish a checksum, no idea what that ASC file is.

mfschumann commented 2 years ago

Actually they do publish the checksums, so you can take it from there: http://ftp.mozilla.org/pub/thunderbird/candidates/102.2.1-candidates/build1/SHA256SUMS

mfschumann commented 2 years ago

I've fixed the fuzz issue, too, please let me know whether you require this in the future.

I will check if I can convince git apply to apply the original patch and let you know about the results.

I was not able to apply the patch in the order prescribed in the series file. When I apply the patch before the Betterbird patches, however, it applies cleanly. Wouldn't it be best anyway to apply the upstream Thunderbird patches first and base the Betterbird modifications on the patched Thunderbird code?

Betterbird commented 2 years ago

New XML committed now, tag coming later: https://github.com/Betterbird/thunderbird-patches/blob/2189b128461778b1c7e8855ceda1c0f9b9a6db4b/metadata/eu.betterbird.Betterbird.appdata.xml#L37

Wouldn't it be best anyway to apply the upstream Thunderbird patches first and base the Betterbird modifications on the patched Thunderbird code?

Yes, but we cherry-pick additional TB patches at the end of the process and add them to the end of the series file since putting them at the front would be penalised with a complete re-compile of 30+ minutes. The most fluctuation is at the end of the series file.

flathubbot commented 2 years ago

Started test build 108744

mfschumann commented 2 years ago

That patch lead to errors in an early version of my build script, but it now works (with warnings, though). I probably don't need to comment it out anymore.

Which warning? Maybe about mixed use of LR and CRLF. The Windows file it creates needs to have CRLF. I can't see anything else wrong with it.

Yes, the warning is about trailing whitespace:

Applying patch 08-branding-m-c.patch ...
patches/08-branding-m-c.patch:16: trailing whitespace.
import subprocess, sys
patches/08-branding-m-c.patch:17: trailing whitespace.

patches/08-branding-m-c.patch:18: trailing whitespace.
p = subprocess.Popen(["C:/Windows/System32/WindowsPowerShell/v1.0/powershell.exe",
patches/08-branding-m-c.patch:19: trailing whitespace.
    "Set-AuthenticodeSignature -FilePath instgen\\setup.exe -Certificate (Get-ChildItem -Path Cert:\CurrentUser\My\ -CodeSigningCert)"],
patches/08-branding-m-c.patch:20: trailing whitespace.
    stdout=sys.stdout)
warning: squelched 21 whitespace errors
warning: 26 lines add whitespace errors.

I am happy to ignore the warning and skip disabling the patch when building the flatpak, so there is no need for you to do anything at the moment.

mfschumann commented 2 years ago

I will merge this pull request because applying external patches works now. Let's continue our discussion on building from source in PR #5

flathubbot commented 2 years ago

Build 108744 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/106393/eu.betterbird.Betterbird.flatpakref

Betterbird commented 2 years ago

Yes, the warning is about trailing whitespace:

There is no trailing whitespace. The line endings must be CRLF for a Windows script and in this context CR is not whitespace. See the editor view:

flathub / eu.betterbird.Betterbird

Download external patches #11