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

Add a function to allow writing old emails into the maildir #39

Closed
wants to merge 2 commits into from
Closed

Add a function to allow writing old emails into the maildir #39

wants to merge 2 commits into from

Conversation

williamdes
Copy link

I used the following documents:

For the pull-request linked below I need to write historical data, this new functions allows to write such data

Note: the MailEntries next function is written in a way that emails that would be in new with flags can not be found.
I find this weird, is there a documented reason ?
See:

maildir/src/lib.rs

Lines 262 to 267 in 3546472

Subfolder::New => (Some(filename.as_str()), Some("")),
Subfolder::Cur => {
let delim = format!("{}2,", INFORMATIONAL_SUFFIX_SEPARATOR);
let mut iter = filename.split(&delim);
(iter.next(), iter.next())
}

Ref: stalwartlabs/mail-server#600

@staktrace
Copy link
Owner

I'm away at the moment, probably won't take a look at this for at least a week or so. Sorry for the wait

@staktrace
Copy link
Owner

Hi @williamdes , in looking at the PR I don't like the fact that this exposes more of maildir internals to the consumer. Stuff like tmp_file_path, tmp_file etc should be abstracted away as implementation detail but this makes it part of the public API. It's also not clear to me why this is needed.

I looked at the PR you referenced in mail-server but it only calls store_new and it's not obvious to me where you would be calling this new function you're adding and how you would be obtaining the arguments for it. In other words, I guess what I'm asking is why does store_cur_with_flags not meet your needs?

Note: the MailEntries next function is written in a way that emails that would be in new with flags can not be found.
I find this weird, is there a documented reason ?

My interpretation of https://cr.yp.to/proto/maildir.html is that entries in new are not supposed to contain flags.

@williamdes

This comment was marked as resolved.

@staktrace
Copy link
Owner

Because it will use a hostname that is the wrong one, and not allow to change the subfolder More or less because it does not export the historical data in a way that makes it coherent to what it really is

The hostname is supposed to be an implementation detail of the uniqueness algorithm. As it says at https://cr.yp.to/proto/maildir.html (emphasis added):

A unique name can be anything that doesn't contain a colon (or slash) and doesn't start with a dot. Do not try to extract information from unique names.

It sounds like you intend to extract the hostname or other parameters from the filename in order to make this historical data "coherent", but that is violating the above. I'm ok with adding a function to indicate the subfolder to save in, because that is part of the maildir structure. But the filename details I'm not really ok with exposing without better justification.

Okay, would a struct suit you ?

Whether it's part of a struct or individually listed arguments doesn't make much difference, it would still be part of the API.

@williamdes

This comment was marked as resolved.

@staktrace
Copy link
Owner

The hostname is supposed to be an implementation detail of the uniqueness algorithm

And it is wrong (value) when executed as a CLI to extract emails from a server since it should have the server hostname not the CLI (client) hostname

But that's my point - the actual hostname used shouldn't really matter, it's just a way to get a unique value.

But the filename details I'm not really ok with exposing without better justification.

Hmm, okay. But in that case your crate can never be used to do export/import operations :/ I mean that with Rust code I should be able to do the following Extract maildir contents -> import into some system -> export the system back into Maildir

The goal would be to achieve near to 100% the same result from the first step compared to the last step

If you are considering the exact filename as semantically important then yes you are correct, that is not a goal of this crate. This crate is aimed at a higher level of abstraction, so the exported maildir would contain the same data in the same folder structure, but the filenames may be different. However no compliant tool should be able to observe a difference.

What should I do know, I feel like this is an ideological dead end that would lead to forking this lib or finding another one. I do not really like this idea of forking. Is there any chance that we can find a way to make my changes part of some API ?

Note, other crates seem to have a set_id method: https://docs.rs/maildirpp/0.3.0/maildirpp/struct.MailEntry.html#method.set_id

Why don't you use that crate then?

@soywod
Copy link
Contributor

soywod commented Jul 30, 2024

I had a second project in mind with this crate, ready my IMAP directory and export the old emails into a Maildir folder for archiving. With the functions I added in this PR it would be possible.

What you need is a synchronizer, sth like OfflineIMAP or mbsync. If your goal was to learn or use Rust, then have a look at email-lib, it's not yet stable but you get everything you need to interact with emails (both IMAP and Maildir). The project used to use this crate, but then switched to maildirpp because it needed a different mail parser (see #35).

@williamdes
Copy link
Author

If you are considering the exact filename as semantically important then yes you are correct, that is not a goal of this crate. This crate is aimed at a higher level of abstraction, so the exported maildir would contain the same data in the same folder structure, but the filenames may be different. However no compliant tool should be able to observe a difference.

Okay, I guess that's it. Thank you for better explaining what are the bounds of this crate.

Merci @soywod for pointing out some other crates to use

Why don't you use that crate then?

I did not find it on my first search ^^

Bye all, thank you for the talk 👋🏻

@williamdes williamdes closed this Jul 30, 2024
@williamdes williamdes deleted the restore_function branch July 30, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants