[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured jhrozek commented: """ * master: fe58f0fbf34de5931ce3305396e5e4467796a325 b8db8c2d83d1d75c42c1e17145d3907211b3a146 """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-380086262 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured
fidencio commented:
"""
CI: http://vm-031.${abc}/logs/job/87/72/summary.html
"""
See the full comment at
https://github.com/SSSD/sssd/pull/515#issuecomment-37911
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured fidencio commented: """ @amitkumar50, I'll just run our internal CI in your patches (for the sake of the process) and then I'll add the Accepted label (in case everything passes) as the patches are matching Pavel's review. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-379730780 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured amitkumar50 commented: """ Huge Thanks @fidencio for tmate session help. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-379730432 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured fidencio commented: """ @amitkumar50, you have squashed **all** patches from @pbrezina into yours and that's not exactly the best way to go. We'd like to keep: Your patches with the two first patches provided by @pbrezina squashed into yours. The third patch should go **atop** of yours, still keeping his patch, his authorship, the commit message and so on. Would be possible to do that? If you don't know how to do that, please, feel free to ping me in a few hours and I can guide you to do so. Best Regards, """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-378488390 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured amitkumar50 commented: """ @jhrozek @pbrezina Done the changes on my branch itself. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-378486902 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured fidencio commented: """ @amitkumar50, please, squash the patches suggested by Pavel and also add his other patch atop of yours. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-378249044 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured pbrezina commented: """ Ack. But please squash these two commits from [1]: 1. coding style fix: https://github.com/pbrezina/sssd/commit/94906199bc0eb70f8134a39d08c3be0df2d43e62 2. move help variable to tool_ctx: https://github.com/pbrezina/sssd/commit/ce8a5fe6eff4d86d026d3a1116cfc28c60a72958 And push this patch along: 3. fix for the error issue a reported here: https://github.com/pbrezina/sssd/commit/5ddd733d45a780a52df211f7ed70267c9ae95218 [1] https://github.com/pbrezina/sssd/commits/sssctl """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-378169552 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured jhrozek commented: """ @pbrezina can you also approve the patches to make sure I don't ack something that you wouldn't like architecturally? """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-372623284 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured jhrozek commented: """ retest this please """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-372606305 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured jhrozek commented: """ I think the tests failed because of the issue fixed in pr #532, so I'm going to reschedule them.. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-372606255 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured amitkumar50 commented: """ @jhrozek Done changes. Thanks """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-372545291 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured amitkumar50 commented: """ @pbrezina Done thanks. But passed "help" value back 3 functions.. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-369913626 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured
pbrezina commented:
"""
You have to let popt to parse the options first, i.e.:
```c
poptContext pc;
int debug = SSSDBG_DEFAULT;
int orig_argc = *argc;
int help = 0;
int opt;
struct poptOption options[] = {
{"debug", '\0', POPT_ARG_INT | POPT_ARGFLAG_STRIP, &debug,
0, _("The debug level to run with"), NULL },
{"help", '?', POPT_ARG_VAL | POPT_ARGFLAG_DOC_HIDDEN, &help,
1, NULL, NULL },
POPT_TABLEEND
};
pc = poptGetContext(argv[0], orig_argc, argv, options, 0);
while ((opt = poptGetNextOpt(pc)) != -1) {
/* do nothing */
}
DEBUG(SSSDBG_CRIT_FAILURE,"\n[Amit] help_value=%d\n", help);
...
```
"""
See the full comment at
https://github.com/SSSD/sssd/pull/515#issuecomment-369870631
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured
amitkumar50 commented:
"""
@pbrezina Thanks.
With /usr/local/sbin/sssctl cache-remove --help
int help = 100;
int opt;
struct poptOption options[] = {
{"debug", '\0', POPT_ARG_INT | POPT_ARGFLAG_STRIP, &debug,
0, _("The debug level to run with"), NULL },
{"help", '?', POPT_ARG_VAL | POPT_ARGFLAG_DOC_HIDDEN, &help,
1, NULL, NULL },
POPT_TABLEEND
};
DEBUG(SSSDBG_CRIT_FAILURE,"\n[Amit] help_value=%d\n",help); //Its o/p
is 0
Looks correct value is not getting filled in parameter 'help'.
Are we using correct `argInfo` and `void * arg` will depend on that.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/515#issuecomment-369824289
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured pbrezina commented: """ In short, `poptGetNextOpt` returns next positioned option without value (`-o`) and returns its name (`o`). For example, see: `init_context()` in `src/tools/sss_cache.c`. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-368819406 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured
pbrezina commented:
"""
No, this is not correct usage of popt. You want something like this:
```c
static void sss_tool_common_opts(struct sss_tool_ctx *tool_ctx,
- int *argc, const char **argv)
+ int *argc, const char **argv,
+ bool *_help)
{
poptContext pc;
int debug = SSSDBG_DEFAULT;
int orig_argc = *argc;
+int help = 0;
int opt;
struct poptOption options[] = {
{"debug", '\0', POPT_ARG_INT | POPT_ARGFLAG_STRIP, &debug,
0, _("The debug level to run with"), NULL },
+{"help", '?', POPT_ARG_VAL | POPT_ARGFLAG_DOC_HIDDEN, &help,
+1, NULL, NULL },
POPT_TABLEEND
};
@@ -74,6 +78,7 @@ static void sss_tool_common_opts(struct sss_tool_ctx
*tool_ctx,
/* Strip common options from arguments. We will discard_const here,
* since it is not worth the trouble to convert it back and forth. */
*argc = poptStrippedArgv(pc, orig_argc, discard_const_p(char *, argv));
+*_help = help;
DEBUG_CLI_INIT(debug);
```
"""
See the full comment at
https://github.com/SSSD/sssd/pull/515#issuecomment-368818331
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured
amitkumar50 commented:
"""
@pbrezina Thanks for review.
As I understood I need to add `poptGetOptArg(poptContext context)` function
inside `poptGetNextOpt()` to parse --help option.
But when I tried adding something as this:
while ((opt = poptGetNextOpt(pc)) != -1) {
char *value = poptGetOptArg(pc);
printf("[Amit] opt=%d\n",opt); //returned -11
printf("[Amit] value=%s\n",value); //returned null
}
Need to spend time for popt library..
"""
See the full comment at
https://github.com/SSSD/sssd/pull/515#issuecomment-368761037
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515
Title: #515: sssctl: Showing help even when sssd not configured
pbrezina commented:
"""
Well, currently I see some one kind of interesting code issue there that should
be fixed as well.
Jakub did first this change:
```c
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 490) int sss_tool_main(int
argc, const char **argv,
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 491)
struct sss_route_cmd *commands,
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 492) void
*pvt)
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 493) {
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 494) struct
sss_tool_ctx *tool_ctx;
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 495) uid_t uid;
e98ccef2 (Pavel Březina 2016-06-09 16:13:34 +0200 496) errno_t ret;
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 497)
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 498) uid = getuid();
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 499) if (uid != 0) {
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 500)
DEBUG(SSSDBG_CRIT_FAILURE, "Running under %d, must be root\n", uid);
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 501) ERROR("%1$s
must be run as root\n", argv[0]);
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 502) return
EXIT_FAILURE;
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 503) }
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 504)
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 505) ret =
sss_tool_init(NULL, &argc, argv, &tool_ctx);
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 506) if (ret ==
ERR_SYSDB_VERSION_TOO_OLD) {
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 507)
tool_ctx->init_err = ret;
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 508) } else if (ret !=
EOK) {
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 509)
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to create tool context\n");
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 510) return
EXIT_FAILURE;
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 511) }
```
But then Michal removed the initialization code from `sss_tool_init` and put it
to `tool_cmd_init` which is called from `sss_tool_route`.
```c
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 328) if
(!sss_tools_handles_init_error(&commands[i], tool_ctx->init_err)) {
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 329)
DEBUG(SSSDBG_FATAL_FAILURE,
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 330)
"Command %s does not handle initialization error [%d] %s\n",
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 331)
cmdline.command, tool_ctx->init_err,
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 332)
sss_strerror(tool_ctx->init_err));
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 333) return
tool_ctx->init_err;
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 334) }
a0b824ac (Jakub Hrozek2016-07-01 13:26:38 +0200 335)
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 336) ret =
tool_cmd_init(tool_ctx, &commands[i]);
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 337) if (ret !=
EOK) {
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 338)
DEBUG(SSSDBG_FATAL_FAILURE,
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 339)
"Command initialization failed [%d] %s\n",
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 340)
ret, sss_strerror(ret));
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 341) return
ret;
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 342) }
cbee11e9 (Michal Židek2016-10-12 13:09:37 +0200 343)
284937e6 (Pavel Březina 2015-07-22 10:02:02 +0200 344) return
commands[i].fn(&cmdline, tool_ctx, pvt);
```
(I'm not pointing figers, I put git blame here just so we can see the git
hash). This rendered Jakub's change basically a dead code, because
`sss_tool_init` only returns `ENOMEM` or `EOK`.
Going back to this ticket. We parse command line for common options in
`sss_tool_init`. We should also parse popt help options here, return some error
code if help option was found and then skip the `tool_cmd_init` or set
`SSS_TOOL_FLAG_SKIP_CMD_INIT` on the comman(s).
"""
See the full comment at
https://github.com/SSSD/sssd/pull/515#issuecomment-367654935
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured jhrozek commented: """ Just by looking at the diff I'm not sure comparing argv with --help is the best approach, but I wonder if @pbrezina has a better idea? """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-367459847 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured lslebodn commented: """ retest this please """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-366440039 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured amitkumar50 commented: """ Don't know why build check failed.. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-366146010 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured amitkumar50 commented: """ ok to test """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-366146010 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured lslebodn commented: """ ok to test """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-365909737 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured amitkumar50 commented: """ @tiran Done changes. Thanks. """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-365862182 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured centos-ci commented: """ Can one of the admins verify this patch? """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-365845133 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#515][comment] sssctl: Showing help even when sssd not configured
URL: https://github.com/SSSD/sssd/pull/515 Title: #515: sssctl: Showing help even when sssd not configured centos-ci commented: """ Can one of the admins verify this patch? """ See the full comment at https://github.com/SSSD/sssd/pull/515#issuecomment-365845134 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
