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

WIP: Implementation xattr attributes for bsd platform #37

Closed
wants to merge 3 commits into from

Conversation

Martinfx
Copy link

@Martinfx Martinfx commented Mar 7, 2020

Hi LinusU,
here are changes for bsd platforms .
The problem is in xattr.h because we have not for FreeBSD and we must use sys/extattr.h with prefix

@Martinfx Martinfx changed the title fixed build for bsd platforms Fixed build for bsd platforms Mar 7, 2020
src/error.c Outdated Show resolved Hide resolved
src/async.c Outdated Show resolved Hide resolved
@LinusU
Copy link
Owner

LinusU commented Mar 8, 2020

Great work! Left some comments

Build is complete but implementation js api not.

Would you mind elaborating on this part? I'm not sure I understand 🤔

@Martinfx
Copy link
Author

Martinfx commented Mar 8, 2020

@LinusU Thank you for comments and ideas. I am porting signal desktop application to FreeBSD

@Martinfx Martinfx changed the title Fixed build for bsd platforms WIP: Fixed build for bsd platforms Mar 8, 2020
@Martinfx
Copy link
Author

Martinfx commented Mar 8, 2020

@LinusU Implementation have issue in tests

  xattr#sync
    ✓ should set an attribute
    ✓ should get an attribute
    1) should list the attributes
    ✓ should remove the attribute
    2) should give useful errors

  xattr#async
    ✓ should set an attribute
    ✓ should get an attribute
    3) should list the attributes
    ✓ should remove the attribute
    4) should give useful errors

  xattr#utf8
    ✓ should set an attribute
    ✓ should get an attribute
    5) should list the attributes
    ✓ should remove the attribute
    6) should give useful errors


  9 passing (41ms)
  6 failing

  1) xattr#sync
       should list the attributes:

      AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert.ok(val.includes(attribute0))

      + expected - actual

      -false
      +true
      
      at Context.<anonymous> (test/xattr.js:37:12)
      at processImmediate (internal/timers.js:444:21)

  2) xattr#sync
       should give useful errors:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

87 !== 93

      + expected - actual

      -87
      +93
      
      at Object.<anonymous> (test/xattr.js:50:14)
      at expectedException (assert.js:650:26)
      at expectsError (assert.js:775:3)
      at Function.throws (assert.js:824:3)
      at Context.<anonymous> (test/xattr.js:47:12)
      at processImmediate (internal/timers.js:444:21)

  3) xattr#async
       should list the attributes:
     Error: path or name points to an invalid address.
  

  4) xattr#async
       should give useful errors:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

87 !== 93

      + expected - actual

      -87
      +93
      
      at Context.<anonymous> (test/xattr.js:101:12)

  5) xattr#utf8
       should list the attributes:
     Error: path or name points to an invalid address.
  

  6) xattr#utf8
       should give useful errors:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

87 !== 93

      + expected - actual

      -87
      +93
      
      at Context.<anonymous> (test/xattr.js:147:12)



npm ERR! Test failed.  See above for more details.
`

@Martinfx Martinfx force-pushed the max-fix-freebsd branch 8 times, most recently from 60e492b to 1a73577 Compare March 15, 2020 16:15
@Martinfx Martinfx requested a review from LinusU March 15, 2020 17:26
src/util.h Outdated Show resolved Hide resolved
src/async.c Show resolved Hide resolved
@Martinfx Martinfx changed the title WIP: Fixed build for bsd platforms WIP: Implementation xattr attributes for bsd platform Mar 29, 2020
@Martinfx
Copy link
Author

Martinfx commented Mar 29, 2020

@LinusU, @asomers, @joshuawarner32 Problem on FreeBSD with xattr is that has sometimes
other behavior than on linux os. And therefore some test failed.
http://www.reactivated.net/patches/mono/1.1.8/extended-attr-transparency.patch

src/async.c Outdated Show resolved Hide resolved
src/async.c Outdated Show resolved Hide resolved
@Martinfx
Copy link
Author

Martinfx commented Mar 29, 2020

@asomers next update after your review.

@asomers
Copy link

asomers commented Mar 29, 2020

But i still feel other behavior on FreeBSD

I'm sorry; I don't understand. What do you mean?

@Martinfx
Copy link
Author

Martinfx commented Mar 29, 2020

@asomers I added link for some patch where is implementation for linux and freebsd. For FreeBSD can be other very similar behavior in API. I have problem with extattr_list_file(). I am sorry.

@asomers
Copy link

asomers commented Mar 29, 2020

@asomers I added link for some patch where is implementation for linux and freebsd. For FreeBSD can be other behavior in API. FreeBSD can have other error code or other behavior for call his function.

I'm sorry, but I still don't understand what you want from me. If you can state your problem as a question about FreeBSD's behavior, then I will do my best to answer it. But I can't debug your program for you; I don't even know Javascript.

@Martinfx
Copy link
Author

Martinfx commented Mar 29, 2020

Yes. you are right. Thank you for review. @LinusU do you have some idea how to fix 3 tests?

@Martinfx Martinfx closed this Mar 30, 2020
@Martinfx Martinfx deleted the max-fix-freebsd branch March 30, 2020 21:00
@LinusU
Copy link
Owner

LinusU commented Mar 31, 2020

Sorry for being slow to respond here, I'll try and look into this some more this weekend!

What seems to be the limitations in the FreeBSD api? That you can't reliably list set attributes?

I'd be happy to help you continue work on this :)

@LinusU
Copy link
Owner

LinusU commented Mar 31, 2020

The different lengths is interesting, could you add some console.log to the test to see exactly what the data contains? That should help us figure out why the two buffers have different length

@Martinfx
Copy link
Author

Martinfx commented Apr 4, 2020

Hi @LinusU , I going to continue for this patch for fs-xattr but now i have not time.
console.log

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.

None yet

3 participants