On (07/09/16 08:46), Fabiano Fidêncio wrote: >On Wed, Sep 7, 2016 at 8:34 AM, Lukas Slebodnik <lsleb...@redhat.com> wrote: >> On (06/09/16 21:38), Fabiano Fidêncio wrote: >>>On Tue, Sep 6, 2016 at 8:49 PM, lslebodn >>><sssd-github-notificat...@fedorahosted.org> wrote: >>>> lslebodn commented on a pull request >>>> >>>> """ >>>> IMHO, it might be better to close this PR. >>>> If we decide to dor support for libini_config < 1.1 or 1.2 >>>> then it will be a different patch anyway. @see my previous comment >>>> """ >>> >>>Please, take a look on Jakub's comment and see what he proposed. >>> >> Do you mean: >>> >>>jhrozek commented on a pull request >>> >>>""" >>>> - Ubuntu 12.04 LTS and older >>> >>>12.04 is often used (for example travis-ci still offers only this >>>distro) and ends its life in 2017. >>> >>>On one hand, it's unlikely that users of LTS distributions will run >>>master, they will probably only run sssd-1-13 or some PPAs, on the other >>>hand, I don't see too much value except for cleaner code. >>> >>>So all in all I suggest we nack this patchset for now and push it when >>>Ubuntu 12.04 goes EOL. >>> >>>Does that sound like a reasonable compromise? >>> >>>""" >> If we decide to drop support for older versions of libini_config >> then we should also remove unnecessary wrappers in src/util/sss_ini.c. >> and replace sss_ini* functions with direct usage of ini_*. >> >> Therefore this patchset will need to be changed. >> IMHO, it will be much simpler to create new patch-set then. >> >> >>>Anyways, I'm fine with closing this PR and opening a new one when we >>>can drop support to libini_config < 1.3. >>> >>>Just for the record, from this whole discussion I could see that >>>dropping support to augeas in order to use libini is something that >>>shouldn't and is not going to happen any time soon >> I do not understand how is this patch-set related to replacing augeas >> with libini_config. > >Well, this patch set is _not_ related to replacing augeas with >libini_config, we just will fall under the same issue in the future >when doing that. >If we introduce a new code that will deal with configuration files the >least I would expect is that old systems could have some benefit from it >in the same way they can have benefit from the current code. Maybe you did not get my point and therefore you get wrong expectations. I do not expect that old systems will benefit from new features. e.g. Currently rhel6 cannot: * validate configuration file because it has old version of libini_config * be integrated with "CIFS Client"[1] because it does not have necessary dependencies ...
>So, if I'm the one writing or reviewing the code with this discussion in mind >I'd opt for something that is present in the major distros and for >sure libini > 1.3 is not. So, why not use augeas instead of it? :-) > Augeas is optional dependency libini_config-1.2 is optional dependency. I cannot see a conflict for replacing it :-) But it also does not make a sense because we should drop manipulation with sssd.conf >> >> a) augeas is an optional dependency >> if BUILD_IFP >> if BUILD_CONFIG_LIB >> pkglib_LTLIBRARIES += libsss_config.la >> b) replacing augeas with libini_config requires minimal version 1.2 >> which is already optionally used in master. >> c) IIRC discussion from devel meeting. Some developers prefer >> to drop feature for manipulation of sssd.conf and write from >> scratch if there will be new requirements. Current version is very >> limited. >> >>>as well and for the >>>very same reasons that this patch wasn't accepted. We could, in the >>>best (or worst?) case scenario support/depend on both due to >>>compatibility with elder systems, but I can't see any advantage on >>>that. >>> >> I do not suggest to support all features in old systems. >> But I would like to support at least minimal feature set (ldap+krb5). >> And parsing configuration file is required for minimal feature set :-) >> > >Sure, sure. > >We wasted a lot of time on this patch set that could have been used >reviewing more useful patches on the queue. >Please, close the PR if you want to. > I'm sorry if you think we wasted a lot of time. I did not want to close PR without proper explanation why I would prefer to keep this optional code in sssd. It would be impolite to close PR without general agreement. LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
