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

Remove an extra line which uses a deleted command. #434

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@yasuoka

yasuoka commented Oct 6, 2018

There is an extra line in tests/gss/check-context.in. It uses a deleted command.

@jaltman

This comment has been minimized.

Member

jaltman commented Oct 6, 2018

If you can please reference in the commit message the change that removed the command.

@yasuoka yasuoka force-pushed the yasuoka:extra-line branch from 00eb732 to 9aa2f0e Oct 8, 2018

@yasuoka yasuoka force-pushed the yasuoka:extra-line branch from 9aa2f0e to 29ba8af Oct 8, 2018

@yasuoka

This comment has been minimized.

yasuoka commented Oct 8, 2018

Updated the comment. I hope it becomes better.

@jaltman

This comment has been minimized.

Member

jaltman commented Oct 8, 2018

@yasuoka, Thank you for updating the commit message.

However, I would like to understand when in the repository history resulted in "klist" no longer being available. "klist" is supposed to be an alias for "heimtools klist".

@nicowilliams, was the addition of "heimtools klist -c $cache" in 95a2ba6 supposed to replace the "klist -c $cache" definition?

If so, I would like to see the commit message for this change say

Remove extraneous definition for "klist" that was supposed to be removed by 95a2ba6.

@yasuoka

This comment has been minimized.

yasuoka commented Oct 9, 2018

92a827d does "klist" => "kcc" and then 2b365b2 does "kcc" => "heimtool". "klist" still exists in ${bindir}, but it doesn't seem to exist in ${srcdir}/kuser any longer.

I noticed this problem since my first commit of #433 didn't pass tests/gss/check-context.
The commit had another problem which causes a check failure, but when I checked check-context.log:

./check-context[140]: ../../kuser/klist: not found

The script also has "not found" error. Note that this doesn't cause a check failure because

    139 # gss_acquire_cred_with_password() must not have side-effects
    140 ${klist} && { eval "$testfailed"; }

the test assumes that "klist" command will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment