[Freeipa-devel] [freeipa PR#699][opened] Fix libkrb5 filename for macOS

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Author: neffs
Title: #699: Fix libkrb5 filename for macOS
Action: opened

PR body:
"""
libkrb5.so.3 is called libkrb5.dylib on macOS


"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/699/head:pr699
git checkout pr699

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: Fix libkrb5 filename for macOS

abbra commented:
"""
Thanks. Do you have IPA client code working on Mac OS X?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292523505

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: Fix libkrb5 filename for macOS

neffs commented:
"""
It connects via RPC and user-show works. Didn't check much further.

I also created an issue: https://pagure.io/freeipa/issue/6850
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292524463

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: Fix libkrb5 filename for macOS

neffs commented:
"""
It connects via RPC and user-show works. Didn't check much further.

I also created an issue: https://pagure.io/freeipa/issue/6850
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292524463

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: Fix libkrb5 filename for macOS

abbra commented:
"""
Ok. Let me look at it next week when I'll have time. Could you please add a short step by step instruction how you configured IPA client on Mac OS X?

"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292527660

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: Fix libkrb5 filename for macOS

abbra commented:
"""
There is a PEP8 error:
PEP-8 errors:

./ipapython/session_storage.py:11:21: E225 missing whitespace around operator


"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292527976

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Author: neffs
Title: #699: Fix libkrb5 filename for macOS
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/699/head:pr699
git checkout pr699

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: Fix libkrb5 filename for macOS

neffs commented:
"""
I added the steps here: https://pagure.io/freeipa/issue/6850
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292532086

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Author: neffs
Title: #699: Fix libkrb5 filename for macOS
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/699/head:pr699
git checkout pr699

[tiran]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: Fix libkrb5 filename for macOS

tiran commented:
"""
@neffs thanks David. Please squash your commits into a single commit (```git rebase -i @~3``` and use fixup on the 2nd and 3rd commit, then git push --force).
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292534577

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Author: neffs
Title: #699: Fix libkrb5 filename for macOS
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/699/head:pr699
git checkout pr699

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Author: neffs
Title: #699: macOS compatibility fixes
Action: edited

Changed field: title
Original value:
"""
Fix libkrb5 filename for macOS
"""

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Author: neffs
Title: #699: ipaclient/ipapython macOS compatibility fixes
Action: edited

Changed field: title
Original value:
"""
macOS compatibility fixes
"""

[neffs]

URL: https://github.com/freeipa/freeipa/pull/699
Author: neffs
Title: #699: ipaclient/ipapython macOS compatibility fixes
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/699/head:pr699
git checkout pr699

[tiran]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

Label: +ack

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

Label: -ack

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

abbra commented:
"""
Please don't set ACK yet, I'm not finished with review.

I do not want to replace fdatasync() with fsync(), this is not correct towards other platforms.
I haven't yet tested this pull request against Mac OS X, so do not set ACK yet.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292862967

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

abbra commented:
"""
Note that we need something similar to https://github.com/untitaker/python-atomicwrites/commit/2bdd9dae62b7434c7b2383ce45fb515bdf70c3c3 to behave properly on Mac OS X.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292864166

[tiran]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

tiran commented:
"""
I wrote that ```fdatasync``` -> ```fsync``` is fine. It's my code after all.

Explanation: fdatasync is a slightly optimized version of fsync that does not flush some metadata to disk, https://linux.die.net/man/2/fdatasync

fdatasync() is similar to fsync(), but does not flush modified metadata unless that metadata is needed in order to allow a subsequent data retrieval to be correctly handled. For example, changes to st_atime or st_mtime (respectively, time of last access and time of last modification; see stat(2)) do not require flushing because they are not necessary for a subsequent data read to be handled correctly. On the other hand, a change to the file size (st_size, as made by say ftruncate(2)), would require a metadata flush.

When I write the code, I chose ```fdatasync``` because ```st_mtime``` isn't strictly required for the cache files. ```fdatasync``` is a micro-optimization that fails under macOS. Instead of making the code even more complicated, I have approved the platform agnostic ```fsync``` syscall. It doesn't hurt to flush all data to disk. The files are rarely written anyway.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292865333

[tiran]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

tiran commented:
"""
No, we don't need to sync the directory. These are cache files. It's only important that we don't have half-written cache files on disk. A missing cache file is fine.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292865960

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

abbra commented:
"""
I still need to test the whole set on Mac OS X myself as we have no way to test that in CI. Thus, this PR will depend on me (or some one else from FreeIPA team) to actually test the code on Mac OS X.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292880002

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

abbra commented:
"""
Ok, so far I cannot build a wheel from git repo on Mac OS X as we have a number of limitations ourselves -- we need to fix our configure to allow just generating enough of `ipasetup.py` and make files to run python wheels code. I'll supply a separate PR for this.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-292927905

[tiran]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

tiran commented:
"""
@abbra is there any reason to delay the merge? I like to get the fixes into 4.5 for the upcoming 4.5.1 release. This commit may not be sufficient for full macOS support, but it's definitely required for macOS support. There is no harm to commit it now and fix remaining issues later.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-295193893

[abbra]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

abbra commented:
"""
Well, given that it is not officially supported yet, go ahead.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-295195255

[pvoborni]

URL: https://github.com/freeipa/freeipa/pull/699
Title: #699: ipaclient/ipapython macOS compatibility fixes

pvoborni commented:
"""
IMO this can be put to 4.5.1 (ipa-4-5 branch) but in order to do it, according to FreeIPA devel processes, it needs to be attached (have a ticket link in commit message) to opened issue in 4.5.1 milestone. Otherwise it will go only to master branch (future 4.6). If this fixes 6850, then it can be reopended for it. Otherwise please [open a new issue](https://pagure.io/freeipa/new_issue) with reasoning.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/699#issuecomment-295209903