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

Switch to use -f instead of -F for sendmail #9961

Merged
merged 3 commits into from
Jan 24, 2020

Conversation

zeripath
Copy link
Contributor

It appears that the sendmail function has been incorrect from its merging. We should use -f not -F.

From http://man7.org/linux/man-pages/man8/sendmail.8.html

      -F full_name
              Set  the  sender  full name. This overrides the NAME environment
              variable, and is used only with messages that have no From: mes-
              sage header.

       -f sender
              Set  the  envelope  sender  address.  This  is the address where
              delivery problems are sent to. With Postfix versions before 2.1,
              the   Errors-To:  message  header  overrides  the  error  return
              address.

Fix #9385

@guillep2k
Copy link
Member

guillep2k commented Jan 24, 2020

It's weird, this have been working fine since #355! How is this possible?

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 24, 2020
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 24, 2020
@codecov-io
Copy link

codecov-io commented Jan 24, 2020

Codecov Report

Merging #9961 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9961      +/-   ##
==========================================
- Coverage   42.24%   42.24%   -0.01%     
==========================================
  Files         610      610              
  Lines       80274    80274              
==========================================
- Hits        33914    33912       -2     
- Misses      42198    42199       +1     
- Partials     4162     4163       +1
Impacted Files Coverage Δ
modules/process/manager.go 74.69% <0%> (-3.62%) ⬇️
models/repo.go 49.69% <0%> (-0.14%) ⬇️
services/pull/patch.go 67.92% <0%> (ø) ⬆️
services/pull/temp_repo.go 34.18% <0%> (+2.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b445d4...0aec996. Read the comment docs.

@zeripath
Copy link
Contributor Author

I suspect it has not been working fine!

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 24, 2020
@6543
Copy link
Member

6543 commented Jan 24, 2020

I think this can be easely backported to 1.10.x too

@zeripath
Copy link
Contributor Author

Make lg-tm work

@zeripath zeripath merged commit 700611c into go-gitea:master Jan 24, 2020
@6543
Copy link
Member

6543 commented Jan 24, 2020

pleace send backport

techknowlogick pushed a commit to techknowlogick/gitea that referenced this pull request Jan 24, 2020
@techknowlogick techknowlogick added the backport/done All backports for this PR have been created label Jan 24, 2020
@zeripath zeripath deleted the sendmail-fixes branch January 24, 2020 23:27
techknowlogick added a commit that referenced this pull request Jan 25, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sendmail does not work / doesn't use FROM parameter
6 participants