linagora / tmail-backend

GNU Affero General Public License v3.0
36 stars 17 forks source link

Update James SHA-1 #932

Closed Arsnael closed 5 months ago

Arsnael commented 6 months ago
[ERROR] Failed to execute goal io.github.git-commit-id:git-commit-id-maven-plugin:7.0.0:revision (get-the-git-infos) on project testing-base: Git command exited with invalid status [128]: directory: `/home/jenkins/build/workspace/Tmail_build_PR-932/.git/modules`, command: `git describe --always --dirty=-dirty --match=* --abbrev=7`, stdout: ``, stderr: `fatal: this operation must be run in a work tree` -> [Help 1]

There is an issue with https://github.com/apache/james-project/commit/88e1ae8beffaa77bd9ca806135de9ae6778614a3

It seems latests versions of the plugin have some conflicts with git worktrees and submodules.

I saw in the commit log history that @vttranlina already did an attempt a year ago to upgrade to version 6 before doing a revert for a similar reason.

However, the issue linked by Tung, that I found as well, has been merged into version 7.0 (used in the plugin update): https://github.com/git-commit-id/git-commit-id-maven-plugin/issues/639

However can see here https://github.com/git-commit-id/git-commit-id-maven-plugin/pull/642#issuecomment-1681031485 that maybe our case has not been handled (the .git in james-project is not a folder but a file).

I think might need to ask James community if we can revert this plugin upgrade for now? And will open an issue as well on that plugin repo to see if we can get support :)

Arsnael commented 6 months ago

And will open an issue as well on that plugin repo to see if we can get support :)

=> https://github.com/git-commit-id/git-commit-id-maven-plugin/issues/701

Arsnael commented 5 months ago

Waiting for this to be merged: https://github.com/apache/james-project/pull/2085

tested locally, it seems to work perfectly :)

vttranlina commented 5 months ago

Maybe need to cherry-pick commit https://github.com/linagora/tmail-backend/pull/909/commits/860983041a674c57fd10d7a824567fdfa2bc8d3b For sure, should wait for ci result after git plugin pr

Arsnael commented 5 months ago

Rebased after the merge on apache for git commit id plugin upgrade

Arsnael commented 5 months ago

https://james-jenkins.lin-saas.com/job/Tmail%20build/job/PR-932/3/testReport/

Related with bouncycastle upgrade.... Not sure what's causing this yet

Arsnael commented 5 months ago

I'm unsure to really understand what's going on...

Tests are failing with : java.io.EOFException: Unexpected end of ZIP input stream

If I change in the Encrypter class PGPCompressedData.ZIP for the PGPCompressedDataGenerator to an other compression like ZLIB or BZIP2 then it works.

I tried to look at if maybe something seems wrong, look around in forums, but not uch. Or maybe there is an issue with zip compression when encrypting in bouncycastle 1.77? I found this https://github.com/bcgit/bc-java/issues/1577 that looks similar but no answers to it.

Also by looking around the net, I tried with just setting .setWithIntegrityPacket(false) in the encryptor generation method and it works. If i understand correctly, the integrity packet contains a hash of the encrypted data, which is then signed with the sender private key, allowing the recipient to verify data has not been tampered with or corrupted.

As technically we encrypt the data with the private key that the user uploaded to tmail, do we need that though? Might be a bad idea, but exploring options here...

Anybody has other ideas? @chibenwa maybe, I think you have a better expertise on that :)

Arsnael commented 5 months ago

Applying here @quantranhong1999 suggestion: forcing back to version 1.70 for tmail part of the build, where we know it works. Could open an issue to try to tackle that back later.

@chibenwa would you be ok with this?

chibenwa commented 5 months ago

@chibenwa would you be ok with this?

Very sad but OK with me.

Though bouncy-castle would be more stable TBH.

Arsnael commented 5 months ago

Though bouncy-castle would be more stable TBH.

Or maybe we do not do something right somewhere... but I've been banging my head a long time on this, it should not be a blocker for now as well I think

Arsnael commented 5 months ago

https://github.com/linagora/tmail-backend/issues/959