[Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Lukas Slebodnik]

On (06/03/17 13:49), Tomas Krizek wrote:
>On 03/06/2017 01:44 PM, Lukas Slebodnik wrote:
>> On (06/03/17 13:35), Tomas Krizek wrote:
>>> On 03/03/2017 09:22 PM, Rob Crittenden wrote:
>>>> Lukas Slebodnik wrote:
>>>>> On (03/03/17 17:07), Lukas Slebodnik wrote:
>>>>>> ehlo,
>>>>>>
>>>>>> This is a small continuation fo discussin from pull request
>>>>>> "Make pylint and jsl optional" #502[1]
>>>>>>
>>>>>> Pylint and jslint are already optional because some downstream distributions
>>>>>> does not have such packages. This is a reason why desing document[2]
>>>>>> mention configuration options for disabling them.
>>>>>>   --disable-pylint --without-jslint
>>>>>>
>>>>>> Previusly (4.4) "pylint was executed" before building rpm packages.
>>>>>> This strict requirement was changed because "make lint" is executed
>>>>>> with each pull request in travis.
>>>>>>
>>>>>> It was changed in commits
>>>>>> master:
>>>>>>
>>>>>> * 5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of pylint
>>>>>> * 3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help message for jslint
>>>>>> * b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock
>>>>>>
>>>>>> The main intention of PR#502 [1] is to make it even more optional
>>>>>> and do not fail if pylint is not installed on machine.
>>>>>> In another words, changing default value from "yes" to "autodetect".
>>>>>> I think the main reason is that it is not obvious that it is an optional
>>>>>> dependency if you run just "./configure". But that can be improved with
>>>>>> better error message. @see attachments.
>>>> I was going to go into a history of why it was required (we pushed
>>>> broken changes into master) but in retrospect that doesn't really
>>>> matter. I've been out of mainline development for some time so don't
>>>> know your current processes, but I do have a question:
>>>>
>>>> Is it expected that ./configure && make && make install will result in
>>>> the bits in all the right places? We never had that expectation before
>>>> though I know Christian has been moving in that direction. Is that an
>>>> end goal? It would be nice for developing in-tree and pushing out micro
>>>> changes onto the current, live development system.
>>> If you provide correct paths to ./configure, yes - make && make install
>>> will place all the bits in the right places. I commonly use it with
>>> DESTDIR and sshfs, so I can develop locally and deploy to a remote
>>> machine without building RPMs.
>>>> If so, does it have checks for all the runtime dependencies or will you
>>>> still have to do a bunch of work afterward the make install?
>>> It doesn't check runtime dependencies. I install the freeipa rpms once
>>> to install dependencies and then use make && make install.
>>>> I've seen discussions about making freeIPA more accessible to the
>>>> average developer, which is good, but it is just so more complex than
>>>> the typical software because it is more about integration than most big
>>>> projects. So I don't know that this is every going to really be true.
>>>> Will it help the average dev install it? Sure, but then what? Will you
>>>> support such an install?
>>>>
>>>> If you want to disable the checks for *lint that is certainly your
>>>> prerogative but I see some downsides:
>>>>
>>>> - I used to setup new dev systems all the time and this is definitely
>>>> something I'd forget to do with some frequency
>>>> - As I understand it the checks will be executed by upstream before a
>>>> change is accepted so that's good but it adds a huge delay and the
>>>> requirement of a roundtrip to fix simple mistakes (happens all the time
>>>> in OpenStack).
>>> On-PR checks can handle this. When you need to fix a linter issue, you
>>> can install the dependencies and run make lint locally.
>>>> I think my question boils down to how many people will this actually
>>>> benefit vs how much time will be lost resubmitting patches? I don't
>>>> think there is an easy answer for the first part but from my own
>>>> experience I'd expect fairly regularly for lint and pep8 errors.
>>> If someone often has this issue, the workflow can be modified to address
>>> it. For example, I've configured my repo to run to run pylint and pep8
>>> on the modified files before the commit.
>>>> On the other hand I guess this also will have the additional advantage
>>>> that make rpms will be significantly faster if you don't enable them.
>>>>
>>>> The --disable vs --without is what bugs me most about the current
>>>> situation :-)
>>>>
>>>> So in closing I'd just say that we made those checks mandatory for a
>>>> reason. Maybe that reason is no longer applicable with all the current
>>>> automation but I'd personally prefer Lukas's suggestion of requiring
>>>> them by default but providing clear output on how to disable them if
>>>> desired. This way the average user can easily work around it and it
>>>> won't impact current developers (unless they want it to). Is that as
>>>> simple as configure; make; make install? No, but it isn't a huge leap
>>>> either.
>>>>
>>>> rob
>>>>
>>> I prefer Christian's approach that makes the project more upstream-friendly.
>>>
>> Could you explain what does "more upstream-friendly" mean?
>> It seems that we have different opinion what does it mean.
>For me, it means making the project easier to develop and install,
>without the need to check ./configure options or having to look for and
>install optional dependencies.

I am sorry but I still did not get your point. Could you a little bit
ellaborate?

And few related questions to the statement "easier to develop and install"

A) All server part is optional. Does it mean that we should disabling server by
default or autodetect all server dependencies and do not build server if they
are missing?

B) it is not just an optional dependency. I tried to explain in 1st mail
that it should be a recomended dependency.

C) Could you explain how it will be easier to develop on debian/other
distribution if upstream does not recommend to run "make lint".

LS