-
-
Notifications
You must be signed in to change notification settings - Fork 661
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
Fix attachment.js error handling with complex archive #3035
Conversation
running |
Looks like you can install bsdtar by adding this to the .github/workflows/ci: ➜ haraka git:(master) ✗ git diff
diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml
index 6eaff2ef..ef4d0d29 100644
--- a/.github/workflows/ci-test.yml
+++ b/.github/workflows/ci-test.yml
@@ -36,6 +36,9 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
+ - name: Install bsdtar
+ run: sudo apt-get install -y bsdtar
+
- name: Install
run: npm install
|
The only thing I've found is that it might be already included https://docs.microsoft.com/en-us/virtualization/community/team-blog/2017/20171219-tar-and-curl-come-to-windows find_bsdtar_path() will need to be updated to find the path inside windows. Sadly I've no access to windows for couple years to check this |
Well, I'm okay with you just putting in a prehook in the attachment plugin tests, and if you can't find bsdtar, then skip the tests. |
I use Windows and I can tell you the following:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, the tests aren't running at all, because plugin.bsdtar_path isn't exported. I suggest changing line 75 from this:
bsdtar_path = `${dir}/bsdtar`;
to this:
plugin.bsdtar_path = bsdtar_path = `${dir}/bsdtar`;
I've tried two blind tests but execution with absolute and relative path was failing, see: (I'll do requested change tomorrow) |
windows tests conversation moved here and deleted from this PR |
Sorry, been out of office. Thanks Matt! |
Fixes # (didn't know that external attachment plugin differs with native attachment.js)
Changes proposed in this pull request:
Checklist: