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

Cyrus and Maven compatibility #3

Closed
Emm opened this issue Feb 26, 2015 · 4 comments
Closed

Cyrus and Maven compatibility #3

Emm opened this issue Feb 26, 2015 · 4 comments

Comments

@Emm
Copy link

Emm commented Feb 26, 2015

Hi,

In order to better support the managesieve protocol in OBM, we are working on a fork of managesievej. This fork includes:

  • Maven support (pom.xml, source code moved to src/main/java)
  • bugfix in getScript (the upstream version does not work properly if the server returns NO/BYE)
  • turned space into tabs (for compatibility with the rest of the codebase)
  • encodeString uses the literal-c2s string representation from RFC 5804 instead of literal-s2c
  • escapeString escapes double quotes
  • Cyrus compatibility: ACTIVATE, GETSCRIPT, PUTSCRIPT and DELETESCRIPT use the quoted string representation for their first argument (Cyrus doesn't seem to parse literal-c2s properly)

For now, the fork has no Netbeans support, as nobody here uses Netbeans (the build.xml and the Netbeans files have been dropped). The work-in-progress XML support has also been dropped.

We would be glad to contribute some of these changes back. What is a bit tricky, though, is that changes like switching the way command encode their strings may break compatibility with other implementations, if they don't conform to the spec in a different way from Cyrus.

@Moosemorals
Copy link
Owner

I'm happy for you to use any of the code (that's why its got a MIT licence!). I'm equally happy to take patches to fix bugs (and RFC incompatibilities are bugs).
I was aware that escapeString() was a very simple implementation - its why I use encodeString() to send to the server. Looking at the RFC, I think that Cyrus is wrong not to accept literal-c2s as the grammar says that "string" is quoted-string, literal-c2s or literal-s2c. On the other hand, since either string representation should work, I can't see an objection to using quoted string (other than the brain-deadness of my escapeString() implementation). I'm testing against dovecot, if that makes any difference....
Netbeans claims to support Maven, although I've never used it. There's already a Mavenised fork here on GitHub but I've never tried it, so don't let me stop you doing your own.
Using tabs instead of spaces may be a dealbreaker :-). (On the other hand, I can re-format anything you send back, so don't worry about it).
I'd suggest you open a bunch of issues for the separate things you've listed, and I'll close this one.
Thanks for the interest in my project!

@Emm
Copy link
Author

Emm commented Feb 27, 2015

On Fri, Feb 27, 2015 at 9:59 AM, Osric Wilkinson notifications@github.com
wrote:

I'm happy for you to use any of the code (that's why its got a MIT
licence!). I'm equally happy to take patches to fix bugs (and RFC
incompatibilities are bugs).
I was aware that escapeString() was a very simple implementation - its
why I use encodeString() to send to the server. Looking at the RFC, I
think that Cyrus is wrong not to accept literal-c2s as the grammar says
that "string" is quoted-string, literal-c2s or literal-s2c.

The RFC also states "This type of literal can only be sent from the server
to the client" when describing literal-s2c. So Dovecot is not compliant in
this regard.

On the other hand, since either string representation should work, I can't
see an objection to using quoted string (other than the brain-deadness of
my escapeString() implementation). I'm testing against dovecot, if that
makes any difference....
Netbeans claims to support Maven, although I've never used it. There's
already a Mavenised fork here on GitHub but I've never tried it, so don't
let me stop you doing your own.

This is actually an important point. Maven expects the code to be in
src/main/java (see the standard directory layout:
http://maven.apache.org/guides/introduction/introduction-to-the-standard-directory-layout.html).
This impacts the submission of patches. However, if moving the files is an
issue for you, it should be possible to patch the pom.xml file to point to
src instead of src/main/java.

Using tabs instead of spaces may be a dealbreaker :-). (On the other hand,
I can re-format anything you send back, so don't worry about it).

Ideally, we would like to see a mavenized ManageSieveJ released on
mvnrepository. This would mean we don't need to keep a fork-with-tabs in
our tree, we could just pull ManageSieveJ as a dependency.

I'd suggest you open a bunch of issues for the separate things you've
listed, and I'll close this one.
Thanks for the interest in my project!

You're welcome. Thanks for making the code available in the first place.

@Moosemorals
Copy link
Owner

Phew! That was a bunch of stuff. I've applied your patches (and thanks again for those), and I'm getting setup to add the project to Maven Central. Anything else on your wishlist?

@Emm
Copy link
Author

Emm commented Mar 4, 2015

That will be all. Thanks for the amazing (and fast) job.

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

No branches or pull requests

2 participants