Fix checking dirs for metainfo.yaml with non-ascii chars with Python 2.7
ClosedPublic

Authored by kossebau on Tue, Jul 30, 6:25 PM.

Details

Summary

Handling projects where metainfo.yaml are then found in paths with
non-ascii chars still needs a separate, more complicated fix.
This patch should though at least unbreak the run of kapidox on api.kde.org

Test Plan

Added a subdir "testü" to kcoreaddons, with the patch python2 kapidox does
no longer fail.

Diff Detail

Repository
R264 KApiDox
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau created this revision.Tue, Jul 30, 6:25 PM
Restricted Application added projects: Frameworks, Documentation. · View Herald TranscriptTue, Jul 30, 6:25 PM
Restricted Application added subscribers: kde-doc-english, kde-frameworks-devel. · View Herald Transcript
kossebau requested review of this revision.Tue, Jul 30, 6:25 PM
pino added a subscriber: pino.Tue, Jul 30, 6:40 PM

Or maybe just drop support for Python 2 entirely?

  • kapidox is rarely used
  • Python 2 will be EOL in 5 months
  • distros either switched kapidox to Python 3, or will do it soon
In D22836#504352, @pino wrote:

Or maybe just drop support for Python 2 entirely?

  • kapidox is rarely used
  • Python 2 will be EOL in 5 months
  • distros either switched kapidox to Python 3, or will do it soon

That as well, as also discussed on irc today.

Just in case that other scripts on api.kde.org do not work yet with Python3 or no-one has time to test that, this patch might allow to at least get the api.kde.org being updated again, which it was not since April for all the repos covered by kapidox.

pino added a comment.Tue, Jul 30, 6:53 PM

Just in case that other scripts on api.kde.org do not work yet with Python3 or no-one has time to test that,

At this point, making Python stuff work with Python3 is priority one. Instead of fixing things for a version of Python that will not be supported anymore in less than a year, I'd say it's much better to make things work with a version of Python that works already, and will work in the future.

In D22836#504356, @pino wrote:

Just in case that other scripts on api.kde.org do not work yet with Python3 or no-one has time to test that,

At this point, making Python stuff work with Python3 is priority one.

Oh dear, any chance we could look at this simple fix provided here, which get us potentially api.kde.org back the day this simple patch is confirmed and in?

Everyone agrees what the better solution is and that somebody(tm) should finally do it.

Just. 3 months have passed since api.kde.org stopped updating for the kapidox covered. Which means, at least 2 releases of KDE Frameworks happened, whose new API is not appearing yet,

So ,any comment on the actiual code change, so we can get passed that? :)

pino added a comment.Tue, Jul 30, 7:12 PM
In D22836#504356, @pino wrote:

Just in case that other scripts on api.kde.org do not work yet with Python3 or no-one has time to test that,

At this point, making Python stuff work with Python3 is priority one.

Oh dear, any chance we could look at this simple fix provided here, which get us potentially api.kde.org back the day this simple patch is confirmed and in?

Oh dear, and what about switching the api generation to Python 3 instead? This fix could "potentially" just buy some more time, and not solve the issue of keep using Python 2 further.

In D22836#504358, @pino wrote:

Oh dear, and what about switching the api generation to Python 3 instead?

Nobody is stopped to work on that. I am not a Python expert, I just providing here the small fix which fall off from me local testing at why the build fails. Which also includes testing whether the bug reproduced locally also is seen with Python3 (it is not). Sorry if I left out this details here, they had been mentioned on Sysadmin.

This fix could "potentially" just buy some more time, and not solve the issue of keep using Python 2 further.

Keeping things broken has not worked to make you switch api.kde.org to Python3, right? ;) I do not think anyone is buyin g time here. It is fixing something which is broken now.

pino added a comment.Tue, Jul 30, 8:01 PM
In D22836#504358, @pino wrote:

Oh dear, and what about switching the api generation to Python 3 instead?

Nobody is stopped to work on that. I am not a Python expert, I just providing here the small fix which fall off from me local testing at why the build fails. Which also includes testing whether the bug reproduced locally also is seen with Python3 (it is not). Sorry if I left out this details here, they had been mentioned on Sysadmin.

This fix could "potentially" just buy some more time, and not solve the issue of keep using Python 2 further.

Keeping things broken has not worked to make you switch api.kde.org to Python3, right? ;)

Surely I cannot work on fix things that I don't know are broken, right? ;)
And surely, I cannot provide more insights if you don't provide more details than "api.kde.org is broken!!!!111oneone", right? ;)
Oh, and shall we stop sneaky remarks, shall not we? ;)

I do not think anyone is buying time here. It is fixing something which is broken now.

it is buying us time. api.kde.org will need to be switched to use Python 3 sooner or later, so let's just take this occasion to do it ASAP, i.e. now. This way, there will be even more time to test things with Python 3 before it will be required to do so in hurry, later on.

pino added a comment.Tue, Jul 30, 8:05 PM

Let me add one more thing: I understand you care about API docs, about making them available for our users on api.kde.org, and about giving the best impression possible about them (I remember various patches about that in the past).

I just don't agree with the "how" in his occasion: we have the solution for this, which is "switch to Python 3". The alternative solution, ie keeping things working on Python 2, is just adding more code for something that people will use less and less in the future, and will leave only api.kde.org the real user of kapidox running on Python 2 in a year or so, which I don't think it is a great idea.

aacid accepted this revision.Tue, Jul 30, 9:08 PM
aacid added a subscriber: aacid.

Pino please take a step back and rethink your position.

Yes, supporting Python2 sucks, but this is 2 lines of code while porting everything to Python 3 is lots of work more.

If this gets us up and running while someone can do the Python 3 there's absolutely no reason not to commit this.

@kossebau maybe add a TODO saying "remove this after pyhon 3 port"?

Other than that if it's confirmed this makes something that is broken work, this should be commited.

This revision is now accepted and ready to land.Tue, Jul 30, 9:08 PM
pino added a comment.Tue, Jul 30, 9:15 PM

Pino please take a step back and rethink your position.
Yes, supporting Python2 sucks, but this is 2 lines of code while porting everything to Python 3 is lots of work more.

kapidox already supports Python 3. Even @kossebau wrote that earlier:

Which also includes testing whether the bug reproduced locally also is seen with Python3 (it is not).

So:

  • kapidox supports Python 2.7, and Python 3
  • the bug is not present when kapidox is run with Python 3
  • api.kde.org runs under Python 2, which is a problem on its own

@aacid: sorry to say that: the one that should rethink their position is you, since you are completely misunderstand what I said, and partially also what Friedrich said.

aacid added a comment.Tue, Jul 30, 9:18 PM

Ok, yep i may have misunderstood :)

But if you say "kapidox supports Python 2.7" we need to commit this to keep it true :)

@kossebau also wrote

That as well, as also discussed on irc today.

Just in case that other scripts on api.kde.org do not work yet with Python3 or no-one has time to test that, this patch might allow to at least get the api.kde.org being updated again, which it was not since April for all the repos covered by kapidox.

I agree that getting api.kde.org uo to date again is a priority. This is a small change to achieve it while testing and confirming everything works with Python 3 takes considerably more time.

pino added a comment.Tue, Jul 30, 9:23 PM

Ok, yep i may have misunderstood :)

But if you say "kapidox supports Python 2.7" we need to commit this to keep it true :)

See my very first comment.

pino added a comment.Tue, Jul 30, 9:26 PM

That as well, as also discussed on irc today.

Just in case that other scripts on api.kde.org do not work yet with Python3 or no-one has time to test that, this patch might allow to at least get the api.kde.org being updated again, which it was not since April for all the repos covered by kapidox.

I agree that getting api.kde.org uo to date again is a priority.

Sure, then switch it to use Python 3. You will fix this problem, and make it work also when Python 2 is EOL and will not be available/supported in distros anymore.

This is a small change to achieve it while testing and confirming everything works with Python 3 takes considerably more time.

It was already written here that using Python 3 works already.

kossebau added a comment.EditedTue, Jul 30, 9:29 PM

I find myself in line with what @aacid said ;)
The whole idea of this patch is to plug in api.kde.org back instantly, so everyone working e.g. on apps for which special new API was added to some KDE Frameworks module can see their API dox.

Switching the whole generation to Python3 needs someone to carefully do this, as there might be unforeseen side-effects when it comes running the wrapper script and doing this over the whole codebase.. This being software after all. Deploy only after testing :)
While the Python2-based system was known to work, there were no code changes otherwie and assumingly generation only started to fall over the non-asccii dirname appearing somewhere.. Which this fix would catch up with, it's effects being easy to oversee, so less surprises to expect.

@bshah being the one who polked api.kde.org some more recently should be the one best positioned to judge, so hoping on his commnents tomorrow :)

In D22836#504361, @pino wrote:

Let me add one more thing: I understand you care about API docs, about making them available for our users on api.kde.org,

For the record, actually I care more for having docs available locally & offline ;) Sadly though there was almot zero feedback on all the work I invested with https://api.kde.org/ecm/module/ECMAddQch.html and their deployment with e.g. all the KDE Frameworks. But well, it's in and I can use the QCH files happily myself in KDevelop, but still curious why no-one else seemed to have interest...

bshah accepted this revision.Wed, Jul 31, 7:22 AM

As a short term workaround, this is fine, in terms of porting to python3 it will also need changes on our server zivo.kde.org which is hosting api.kde.org, right now every scripts including krazy2 and apidox generation scripts are too much complex, and current system is running Gentoo. So overall this asks for much more effort. https://invent.kde.org/branislavaz/documentation-generator is one of steps to tidify all this and get things working properly.

For now I am in favor of submitting this.

lbeltrame accepted this revision.Wed, Jul 31, 7:59 AM
lbeltrame added a subscriber: lbeltrame.

Not the prettiest, but I've seen worse. ;)

This revision was automatically updated to reflect the committed changes.