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.

Sign up for GitHub

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

Jump to bottom

Update fetchmail to selfbuilt 7.0.0-alpha #891

Merged
merged 3 commits into from
Jun 25, 2019
Merged

Update fetchmail to selfbuilt 7.0.0-alpha #891

merged 3 commits into from
Jun 25, 2019

Conversation

Nebukadneza
Copy link
Member

What type of PR?

enhancement

What does this PR do?

Fetchmail in alpine is ~5 years old — and doesn’t support current SSL/TLS
variants anymore. This especially leads to our own fetchmail not being able to
pull mail from mailu itself. Since no new fetchmail release is on the horizon,
let’s build the lastest distribution artifact — which strangely is not
6.4.0-snapshot, but 7.0.0-alpha — ourselves.

Related issue(s)

closes #808

Prerequistes

  • In case of feature or enhancement: documentation updated accordingly
  • Unless it's docs or a minor change: place entry in the changelog, under the latest un-released version.

@Nebukadneza Nebukadneza added type/enhancement Enhances existing functionality status/blocked This will block mergify until the label is removed. review/need2 This will block Mergify untill 2 reviews are done labels Jan 27, 2019
@muhlemmer
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jan 29, 2019
@bors
Try #891:
c375f94
@bors
Copy link
Contributor

bors bot commented Jan 29, 2019

try

Build succeeded

@muhlemmer
Copy link
Member

I tried this and it seems to work okay 👍
I bombed the account to be fetched with 1000 mails, which runs a few minutes and fetchmail timeout set to 60 seconds. And no errors occurred.

Fetched account was on test.mailu.io and the fetchmail instance on my test server.

I do have to note that using the STARTTLS port (143) didn't work. Don't know if this a fetchmail thing or the way the python script is launching fetchmail. Or maybe we never intended STARTTLS?

I would not have anything against merging this. Fetchmail-6.3 is broken. This seems to work. The known bugs in https://gitlab.com/fetchmail/fetchmail/blob/master/NEWS don't seem to affect us. It's just that it's alpha so not very tested by a bigger audience.

I'm open for a second opinion from anybody here. @kaiyou, @Mailu/contributors?

@Nebukadneza
Copy link
Member Author

Again, sorry for insane delay. After looking at the fetchmail source again, i think you’re right — wrapped isn’t right. I guess either not setting an sslmode or setting it to starttls= either may or must would be right. I didn’t fully follow the codepath for may yet, and am unsure what happens on a ssl-only connection/port in that case. I’ll report back … hopefully a bit quicker.

@Nebukadneza Nebukadneza removed the status/blocked This will block mergify until the label is removed. label Mar 10, 2019
@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2019

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

@bors
Copy link
Contributor

bors bot commented Mar 10, 2019

try

Merge conflict

@Nebukadneza Nebukadneza changed the title Experiment: Update fetchmail to selfbuilt 7.0.0-alpha Update fetchmail to selfbuilt 7.0.0-alpha Mar 10, 2019
@Nebukadneza
Update fetchmail to selfbuilt 7.0.0-alpha
5f4a6cf 
Fetchmail in alpine is ~5 years old — and doesn’t support current SSL/TLS
variants anymore. This especially leads to our own fetchmail not being able to
pull mail from mailu itself. Since no new fetchmail release is on the horizon,
let’s build the lastest distribution artifact — which strangely is not
6.4.0-snapshot, but 7.0.0-alpha — ourselves.
@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2019

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

@Nebukadneza
Copy link
Member Author

Hi,

after a longer-than-expected while of dogfooding on this, it seems stable enough to me. Sorry for the force-push, but to resolve the merge conflict in regard to towncrier, actually rebasing seemed best to me.

bors bot added a commit that referenced this pull request Mar 10, 2019
@bors
Try #891:
e5663d2
@bors
Copy link
Contributor

bors bot commented Mar 10, 2019

try

Build succeeded

kaiyou
kaiyou previously approved these changes May 7, 2019
View reviewed changes
Copy link
Member

@kaiyou kaiyou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This builds and runs properly at home

@kaiyou
Copy link
Member

kaiyou commented May 9, 2019

@hoellen do you have time to review this?

hoellen
hoellen previously requested changes May 9, 2019
View reviewed changes
Copy link
Member

@hoellen hoellen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for the one suggestion, the rest looks good to me. In the long run we should use another project. There is not much activity in the fetchmail project. But right now it fixes the current broken setup.

services/fetchmail/Dockerfile Outdated Show resolved Hide resolved
@kaiyou
Copy link
Member

kaiyou commented Jun 24, 2019

@Nebukadneza, any feedback on the suggestion?

@Nebukadneza @hoellen
Add -L to curl to support sourceforge redirects
94dbddd 
As per @hoellens suggestion

Co-Authored-By: hoellen <hoellen@users.noreply.github.com>
@Nebukadneza
Copy link
Member Author

Sorry, had lost track of it :( … makes sense, accepted it ^_^

@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2019

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

@mergify mergify bot dismissed stale reviews from kaiyou and hoellen June 24, 2019 14:08

Pull request has been modified.

bors bot added a commit that referenced this pull request Jun 24, 2019
@bors
Try #891:
a98d389
@bors
Copy link
Contributor

bors bot commented Jun 24, 2019

try

Build failed

@mergify
Copy link
Contributor

mergify bot commented Jun 24, 2019

Thanks for submitting this pull request.
Bors-ng will now build test images. When it succeeds, we will continue to review and test your PR.

bors try

Note: if this build fails, read this.

bors bot added a commit that referenced this pull request Jun 24, 2019
@bors
Try #891:
3aff8dd
@bors
Copy link
Contributor

bors bot commented Jun 24, 2019

try

Build succeeded

@mergify
Copy link
Contributor

mergify bot commented Jun 25, 2019

bors r+

bors bot added a commit that referenced this pull request Jun 25, 2019
@bors @Nebukadneza
Merge #891
829441d 
891: Update fetchmail to selfbuilt 7.0.0-alpha r=mergify[bot] a=Nebukadneza

## What type of PR?
enhancement

## What does this PR do?
Fetchmail in alpine is ~5 years old — and doesn’t support current SSL/TLS
variants anymore. This especially leads to our own fetchmail not being able to
pull mail from mailu itself. Since no new fetchmail release is on the horizon,
let’s build the lastest distribution artifact — which strangely is not
6.4.0-snapshot, but 7.0.0-alpha — ourselves.

### Related issue(s)
closes #808 

## Prerequistes
- [x] In case of feature or enhancement: documentation updated accordingly
- [x] Unless it's docs or a minor change: place entry in the [changelog](CHANGELOG.md), under the latest un-released version.


Co-authored-by: Dario Ernst <dario@kanojo.de>
Co-authored-by: Nebukadneza <github@kanojo.de>
@bors
Copy link
Contributor

bors bot commented Jun 25, 2019

Build succeeded

@bors bors bot merged commit 5ceedde into Mailu:master Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaiyou kaiyou kaiyou approved these changes

@hoellen hoellen hoellen approved these changes

Assignees
No one assigned
Labels
review/need2 This will block Mergify untill 2 reviews are done type/enhancement Enhances existing functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fetchmail sslproto configuration (typo?)
4 participants
@Nebukadneza @muhlemmer @kaiyou @hoellen