[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (opened)

[fidencio]

celestian's pull request #26: "KRB5: Fixing FQ name of user in krb5_setup()" was opened

PR body:
"""
This patch fixes creation of FQ username if krb5_map_user option
ise used.

Resolves:
https://fedorahosted.org/sssd/ticket/3188
"""

See the full pull-request at https://github.com/SSSD/sssd/pull/26
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (comment)

[fidencio]

jhrozek commented on a pull request

"""
Why not do it the other way around and qualify all names in the mapping? That needs to be done only once unlike this approach that needs to unqualify the names every time they are used.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247009644

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (+Changes requested)

[fidencio]

celestian's pull request #26: "KRB5: Fixing FQ name of user in krb5_setup()" label *Changes requested* has been added

See the full pull-request at https://github.com/SSSD/sssd/pull/26

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (synchronize)

[fidencio]

celestian's pull request #26: "KRB5: Fixing FQ name of user in krb5_setup()" was synchronize

See the full pull-request at https://github.com/SSSD/sssd/pull/26
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (comment)

[fidencio]

celestian commented on a pull request

"""
I pushed patch to our CI, we will see result soon.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247304195

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (comment)

[fidencio]

celestian commented on a pull request

"""
http://sssd-ci.duckdns.org/logs/job/53/43/summary.html
So, it is problem with cflags in test. I will send patch soon.

"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247333335

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (synchronize)

[fidencio]

celestian's pull request #26: "KRB5: Fixing FQ name of user in krb5_setup()" was synchronize

See the full pull-request at https://github.com/SSSD/sssd/pull/26
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (-Changes requested)

[fidencio]

celestian's pull request #26: "KRB5: Fixing FQ name of user in krb5_setup()" label *Changes requested* has been removed

See the full pull-request at https://github.com/SSSD/sssd/pull/26

[sssd PR#26] KRB5: Fixing FQ name of user in krb5_setup() (comment)

[fidencio]

celestian commented on a pull request

"""
Link to our CI:
http://sssd-ci.duckdns.org/logs/job/53/46/summary.html
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247537214

[sssd PR#26][+Changes requested] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: +Changes requested

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

jhrozek commented:
"""
You also need to use the output name for `kr->kuserok_user`, currently it's using qualified name, which is not going to work (see the branch just below..)
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247593387

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

lslebodn commented:
"""
Bump,
there are users waiting for this fix.

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247933120

[sssd PR#26][synchronized] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Author: celestian
 Title: #26: KRB5: Fixing FQ name of user in krb5_setup()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

celestian commented:
"""
Hi,
I push new version. All comments are addressed.
Petr
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247961297

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

lslebodn commented:
"""
the following change looks werd to me
`-    dbus_free_string_array(arr_object_path);`
`+    dbus_free_string_array(discard_const(arr_object_path));`

What about following change
`@@ -673,8 +674,8 @@ START_TEST(test_marshal_basic_types)`
`     dbus_int64_t v_int64[] = { INT64_C(-6666666666666666), INT64_C(7777777777777777) };`
`     dbus_uint64_t v_uint64[] = { UINT64_C(7777777777777777), INT64_C(888888888888888888) };`
`     double v_double[] = { 1.1, 2.2, 3.3 };`
`-    char *v_string[] = { "bears", "bears", "bears" };`
`-    char *v_object_path[] = { "/original", "/original" };`
`+    const char *v_string[] = { "bears", "bears", "bears" };`
`+    const char *v_object_path[] = { "/original", "/original" };`

`     unsigned char *arr_byte = v_byte;`
`     dbus_int16_t *arr_int16 = v_int16;`
`@@ -684,8 +685,8 @@ START_TEST(test_marshal_basic_types)`
`     dbus_int64_t *arr_int64 = v_int64;`
`     dbus_uint64_t *arr_uint64 = v_uint64;`
`     double *arr_double = v_double;`
`-    char **arr_string = v_string;`
`-    char **arr_object_path = v_object_path;`
`+    char **arr_string = discard_const_p(char *, v_string);`
`+    char **arr_object_path = discard_const_p(char *, v_object_path);`

BTW the current order of patches would break bisec.
the 1st patch should contain fixes in sbus testa
and then shoudl be added AM_CFLAGS

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247964501

[sssd PR#26][synchronized] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Author: celestian
 Title: #26: KRB5: Fixing FQ name of user in krb5_setup()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

celestian commented:
"""
I change the order of patches, thanks Lukas.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247965473

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

lslebodn commented:
"""
On (19/09/16 04:07), celestian wrote:
> I change the order of patches, thanks Lukas.

Thank you for changing the order but I cannot see requested changes
in src/tests/sbus_codegen_tests.c

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247965818

[sssd PR#26][synchronized] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Author: celestian
 Title: #26: KRB5: Fixing FQ name of user in krb5_setup()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

celestian commented:
"""
Lukas,
sorry,I missed it. New version pushed.

I have question, is there any way how to reply to your comment? This is new one, it is not direct answer.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247970798

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

celestian commented:
"""
Lukas,
sorry,I missed it. New version pushed.

I have a question, is there any way how to reply to your comment? This is new one, it is not direct answer.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247970798

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

lslebodn commented:
"""
On (19/09/16 04:39), celestian wrote:
> Lukas,
> sorry,I missed it. New version pushed.
>
> I have question, is there any way how to reply to your comment? This is new one, it is not direct answer.

The first 2 patches pushed to master:
* 4f2509f8d23d9e921f07b2ead63392ae82ad3a38
* 626d8217a2e578ba641ae3c968752aa15284a210

They were not very related to the ticket #3188

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-247974691

[sssd PR#26][-Changes requested] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: -Changes requested

[sssd PR#26][+Changes requested] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: +Changes requested

[sssd PR#26][synchronized] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Author: celestian
 Title: #26: KRB5: Fixing FQ name of user in krb5_setup()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

celestian commented:
"""
New version is pushed.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-248295259

[sssd PR#26][-Changes requested] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: -Changes requested

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

lslebodn commented:
"""
Bump for review. Users are waiting.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-248548783

[sssd PR#26][+Changes requested] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: +Changes requested

[sssd PR#26][synchronized] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Author: celestian
 Title: #26: KRB5: Fixing FQ name of user in krb5_setup()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26][synchronized] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Author: celestian
 Title: #26: KRB5: Fixing FQ name of user in krb5_setup()
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

celestian commented:
"""
So, all notes addressed (fq_name too).
I pushed new version.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-248580833

[sssd PR#26][-Changes requested] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: -Changes requested

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

lslebodn commented:
"""
Bump for review. Users are waiting.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-248838420

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

jhrozek commented:
"""
Code-wise ACK, the code works and looks good. I will push once Coverity and CI finish.
"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-248858980

[sssd PR#26][+Accepted] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: +Accepted

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

jhrozek commented:
"""
On Thu, Sep 22, 2016 at 02:57:27AM -0700, Jakub Hrozek wrote:
> Code-wise ACK, the code works and looks good. I will push once Coverity and CI finish.

CI: http://sssd-ci.duckdns.org/logs/job/53/87/summary.html

ACK

"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-248881293

[sssd PR#26][comment] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

lslebodn commented:
"""
On (22/09/16 04:48), Jakub Hrozek wrote:
> On Thu, Sep 22, 2016 at 02:57:27AM -0700, Jakub Hrozek wrote:
> > Code-wise ACK, the code works and looks good. I will push once Coverity and CI finish.
>
> CI: http://sssd-ci.duckdns.org/logs/job/53/87/summary.html
>
> ACK

master:
* b34ffbf33729c557c3d1aebf4707ad0ffe4f1904

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/26#issuecomment-249015211

[sssd PR#26][closed] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Author: celestian
 Title: #26: KRB5: Fixing FQ name of user in krb5_setup()
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/26/head:pr26
git checkout pr26

[sssd PR#26][-Accepted] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: -Accepted

[sssd PR#26][+Pushed] KRB5: Fixing FQ name of user in krb5_setup()

[fidencio]

URL: https://github.com/SSSD/sssd/pull/26
Title: #26: KRB5: Fixing FQ name of user in krb5_setup()

Label: +Pushed