On Wed, Sep 7, 2016 at 9:03 AM, Lukas Slebodnik <[email protected]> wrote: > On (07/09/16 08:46), Fabiano Fidêncio wrote: >>On Wed, Sep 7, 2016 at 8:34 AM, Lukas Slebodnik <[email protected]> wrote: >>> On (06/09/16 21:38), Fabiano Fidêncio wrote: >>>>On Tue, Sep 6, 2016 at 8:49 PM, lslebodn >>>><[email protected]> 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
Okay, okay, > >>> >>> 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. There's no reason to be sorry for. > 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. I don't think we are going to agree here. So, please, just close the PR. Best Regards, -- Fabiano Fidêncio _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
