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

mailcatcher: 0.6.5 -> 0.7.1 #56575

Merged
merged 1 commit into from Mar 29, 2019
Merged

mailcatcher: 0.6.5 -> 0.7.1 #56575

merged 1 commit into from Mar 29, 2019

Conversation

zarelit
Copy link
Member

@zarelit zarelit commented Mar 1, 2019

Motivation for this change

The new release of mailcatcher finally has proper UTF-8 support

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.

@aanderse
Copy link
Member

@zarelit looking good. I just built your branch locally and ran it on NixOS stable, all good. Tested both the executable files provided and everything functioned as expected.

Do you have any interest in adding a module for this? A module would be pretty convenient for test boxes, plus a NixOS test would make confirmation the package is working automatic and easier for merging.

@zarelit
Copy link
Member Author

zarelit commented Mar 26, 2019

@aanderse Thank you for the review!
I actually use the derivation on non-nixos machines but I'm interested in writing tests and a module.
What use case do you have in mind for a services.mailcatcher module? Local testing?

@aanderse
Copy link
Member

Local testing primarily, but redirecting mail that all the test copy of production web sites on my nixops dev network generates, and then giving developers who use the network access to the mailcatcher web ui might be useful too.

If you end up writing a test and/or module please ping me as I'm interested.

@aanderse
Copy link
Member

So I started talking to the guys at work and realized this would have way more value than I initially thought. Pretty straight forward first stab at it, but here we go:

master...aanderse:mailcatcher

@Ma27
Copy link
Member

Ma27 commented Mar 28, 2019

@GrahamcOfBorg test mailcatcher

@Ma27
Copy link
Member

Ma27 commented Mar 28, 2019

ofBorg seems to apply this properly on the current master, I get the same error locally when rebasing your branch onto master (7e8d125 in my case):

machine# [   11.241912] mailcatcher[782]: Starting MailCatcher
machine: running command: nc -z localhost 1025
machine: exit status 1
(0.03 seconds)
machine# [   11.820546] mailcatcher[782]: ==> smtp://127.0.0.1:1025
machine# [   12.069383] mailcatcher[782]: /nix/store/f5nbyswzirqqiwgbq7f42slslyw1args-ruby2.5.5-thin-1.5.1/lib/ruby/gems/2.5.0/gems/thin-1.5.1/lib/thin/server.rb:104: warning: constant ::Fixnum is deprecated
machine# [   12.081162] mailcatcher[782]: ==> http://127.0.0.1:1080/
machine: running command: nc -z localhost 1025
machine# Connection to localhost 1025 port [tcp/blackjack] succeeded!
machine: exit status 0
(0.02 seconds)
(2.15 seconds)
machine: must succeed: echo "this is the body of the email" | mail -s "subject" root@example.org
machine# [   13.992932] mailcatcher[782]: ==> SMTP: Received message from '<root@machine>' (277 bytes)
machine# [   13.999045] sSMTP[802]: Sent mail for root@machine (221 Ok) uid=0 username=root outbytes=360
machine: exit status 0
(1.31 seconds)
machine: must succeed: curl http://localhost:1080/messages/1.json
machine: exit status 0
(0.10 seconds)
error: Died at (eval 19) line 6, <__ANONIO__> line 645.
(16.17 seconds)
Died at (eval 19) line 6, <__ANONIO__> line 645.
cleaning up
killing machine (pid 594)
(0.00 seconds)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
builder for '/nix/store/26fd8x881ih72vbkhsa1pj50z2cm88ag-vm-test-run-mailcatcher.drv' failed with exit code 255
error: build of '/nix/store/26fd8x881ih72vbkhsa1pj50z2cm88ag-vm-test-run-mailcatcher.drv' failed

@aanderse
Copy link
Member

Will investigate.

@zarelit
Copy link
Member Author

zarelit commented Mar 28, 2019

Also, I'd like to avoid having GCC in the mailcatcher closure but I think it happens at a lower level (e.g. in the bundlerApp code) and thus it might be good for another PR/issue

@zarelit
Copy link
Member Author

zarelit commented Mar 28, 2019

@Ma27 @aanderse thank you for working on getting this merged!

@aanderse
Copy link
Member

@zarelit I noticed mailcatcher 0.7.1 has been released. Can you please bump?
@Ma27 Incompatible change in the web server from 0.6 series to 0.7 series. Actual email message content no longer available under /messages/1.json, but instead /messages/1.source.

#58489

@zarelit zarelit changed the title mailcatcher: 0.6.5 -> 0.7.0 mailcatcher: 0.6.5 -> 0.7.1 Mar 28, 2019
@Ma27
Copy link
Member

Ma27 commented Mar 29, 2019

@GrahamcOfBorg test mailcatcher

@Ma27 Ma27 merged commit 34fd0cc into NixOS:master Mar 29, 2019
@Ma27
Copy link
Member

Ma27 commented Mar 29, 2019

@zarelit thanks!

@zarelit zarelit deleted the upgrade_mailcatcher branch June 25, 2022 15:00
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.

None yet

4 participants