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

thunderbird: clean up, make flags closer to default, official branding #55026

Closed
wants to merge 1 commit into from

Conversation

@yegortimoshenko
Copy link
Member

@yegortimoshenko yegortimoshenko commented Feb 1, 2019

Motivation for this change

Clean up derivation, strip away flags that upstream is not happy with (such as --disable-alsa), enable official Thunderbird branding.

Superset of #38943, #36449.

cc @jerith666 @vcunat

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@yegortimoshenko
Copy link
Member Author

@yegortimoshenko yegortimoshenko commented Feb 1, 2019

@sylvestre: Could you look at this again?

Section with flags is:

"--enable-application=comm/mail"
"--enable-default-toolkit=cairo-gtk3"
"--enable-jemalloc"
"--enable-js-shell"
"--enable-rust-simd"
"--enable-startup-notification"
"--disable-alsa"
"--disable-crashreporter"
"--disable-gconf"
"--disable-tests" # tests require network access
"--disable-updater"
"--enable-system-ffi"
"--enable-system-hunspell"
"--enable-system-pixman"
"--enable-system-sqlite"
"--with-system-bz2"
"--with-system-icu"
"--with-system-jpeg"
"--with-system-libevent"
"--with-system-png"
"--with-system-zlib"
"--with-clang-path=${llvmPackages.clang}/bin/clang"
"--with-libclang-path=${llvmPackages.libclang}/lib"
] ++ optional enableCalendar "--enable-calendar"
++ optional enableOfficialBranding "--enable-official-branding"
++ (if debugBuild then [ "--enable-debug"
"--enable-profiling"]
else [ "--disable-debug"
"--enable-release"
"--disable-debug-symbols"
"--enable-optimize"
"--enable-strip" ]);

This has been largely based off Debian mozconfig, including vendored NSS/NSPR. We can't run tests since all builds are sandboxed, and other system libs contain Nix-specific patches.

@ryantm
Copy link
Member

@ryantm ryantm commented Feb 26, 2019

@yegortimoshenko Is this waiting on review from Mozilla?

@vcunat
Copy link
Member

@vcunat vcunat commented Mar 1, 2019

@ryantm: the enableOfficialBranding ? true part should work wait for them, I believe. The rest can most likely be done independently.

yegortimoshenko added a commit to yegortimoshenko/yegortimoshenko-flake that referenced this pull request Aug 4, 2019
@matthewbauer matthewbauer modified the milestones: 19.03, 20.03 Aug 28, 2019
@lovesegfault lovesegfault mentioned this pull request Dec 9, 2019
4 of 10 tasks complete
@lovesegfault
Copy link
Contributor

@lovesegfault lovesegfault commented Dec 9, 2019

I suggest this be superseded by #75328

@ryantm
Copy link
Member

@ryantm ryantm commented Dec 11, 2019

Sure. yegortimoshenko can always reopen if he disagrees.

@ryantm ryantm closed this Dec 11, 2019
@vcunat
Copy link
Member

@vcunat vcunat commented Dec 12, 2019

We did not advance the issue branding by default. @sylvestre: can you suggest how to proceed with that? (I.e. whether it's OK or what's the problem with our current state. Or how to find this out.)

@jerith666
Copy link
Contributor

@jerith666 jerith666 commented Aug 17, 2020

for the record, official branding was enabled in #94880.

@sylvestre
Copy link

@sylvestre sylvestre commented Aug 18, 2020

@jerith666 could you please tell me who reviewed the nix patches on top of tb?
Thanks

@jerith666
Copy link
Contributor

@jerith666 jerith666 commented Aug 18, 2020

I don't know -- I only noticed #94880 because I had official branding enabled locally and got a merge conflict with it. You've already commented on #94880 so I think you probably know more than I do. :)

@vcunat vcunat mentioned this pull request Aug 18, 2020
3 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.