-
Notifications
You must be signed in to change notification settings - Fork 17
Fix no attachment with GNU Mailutils #10
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
base: master
Are you sure you want to change the base?
Conversation
Use mailx-wrapper.sh as a thin wrapper around GNU Mailutils mailx.
Behavior:
- Passes through all args to mailx unchanged
- If invoked with -V anywhere: prints a custom string and exits
- If invoked with --smtp <FILE>:
* Reads ALL SMTP-related settings from FILE (a simple key=value format)
* Forces SMTP transport via --exec 'set sendmail="smtp://...";'
| CUPS_BACKEND_DIR?=/usr/lib/cups/backend | ||
| PS2PDF?=/usr/bin/epstopdf | ||
| MAILX?=/usr/bin/mailx | ||
| MAILX?=$(CUPS_BACKEND_DIR)/mailx-wrapper.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like the default mailx to remain /usr/bin/mailx. Cups filters are fragile enough as is, so the default should remain the most simple codepath. This variable is for the install checks, so I'd just add an extra install check for the wrapper script.
| | $Mailx $mailrc_opt -n -r '$MailFrom' -s '$MailSubject' '|' Job:'$1' $attach_opt '$MAILPDF' $MailTo" 1>&2 | ||
| _mailx_status=$( | ||
| echo "$MailBody" \ | ||
| | $Mailx $mailrc_opt -n -r "$MailFrom" -s "$MailSubject | Job:$1" $attach_opt "$MAILPDF" "$MailTo" 2>&1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mostly looks OK, but default output format should not change. So appending the job information into the subject as default is not OK. It makes sense to have that, though - but there might be other useful info in the subject as well, which can get messy quickly. So my suggestion is to drop that from this pull request, and create a new issue about adding extra information into the subject line.
| *DefaultImageableArea: Letter | ||
| *DefaultPaperDimension: Letter | ||
| *DefaultImageableArea: A4 | ||
| *DefaultPaperDimension: A4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This (and the ones above) should remain Letter as default. That's the kind of setting that should be adjusted after deployment.
| *OpenUI MailTo/E-Mail Address: PickOne | ||
| *OrderDependency: 10 AnySetup *MailTo | ||
| *DefaultMailTo: None | ||
| *DefaultMailTo: Custom.mobileprint@scientificnet.eu.uniflowonline.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should stay as None - I assume that was an accident?
| *% configuration for mailx, mailrc | ||
| *OpenUI *mailx_MAILRC/Path to mailrc: PickOne | ||
| *Defaultmailx_MAILRC: Default | ||
| *Defaultmailx_MAILRC: Custom.mailx-wrapper.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should stay default, with documentation added about using the mailx wrapper
| @@ -0,0 +1,7 @@ | |||
| host=smtp.doma.in | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use example.com for example domains - that's specifically assigned by IANA for that purpose
| # specific option(s) to use | ||
| case $_mailx_version in | ||
| *"mailx-wrapper"*) | ||
| export -n MAILRC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That probably should happen inside the wrapper script?
| case $_mailx_version in | ||
| *"mailx-wrapper"*) | ||
| export -n MAILRC | ||
| mailrc_opt="--smtp $MAILRC" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong indent, and also, not sure if needed - we should have the path to mailrc in $MAILRC, so the script should just use that, like the real mailx
| @@ -0,0 +1,182 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should just be #!/bin/bash
#5 seems to have reappeared and (the original) fix #7 seems to be the culprit:
Ubuntu ship (as one possibility) mailx as part of the GNU mailutils, for example
but the syntax for mailx (GNU Mailutils) 3.14 says:
In fact, even back in Ubuntu 14.04LTS this has been the syntax for GNU Mailutils. So, I'm a bit surprised this fix ever worked as intended... In any case, currently the version doesn't work with Ubuntu/GNU Mailutils.
Now, I
mailx -Vstring and, accordingly, set$attach_opt