Page MenuHomePhabricator

Windows: remove fnmatch in Evil and use the one in regex DLL installed by ewpi
AbandonedPublic

Authored by vtorri on Apr 6 2019, 8:40 PM.

Details

Summary

simplify Evil code by replacing its fnmatch code by the one in ewpi regex one

Test Plan

configuration

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10783
Build 8372: arc lint + arc unit
vtorri created this revision.Apr 6 2019, 8:40 PM
vtorri requested review of this revision.Apr 6 2019, 8:40 PM
vtorri edited the summary of this revision. (Show Details)Apr 6 2019, 8:43 PM
vtorri added a reviewer: bu5hm4n.
zmike added a comment.Apr 8 2019, 9:12 AM

It seems to me like it should be possible to greatly simplify the meson parts by just adding regexp to the eina deps so that it gets pulled into everything else without being explicitly added?

that's my way of doing things : i add symbols only when needed.
if it is the way t go (adding a lib in eina so that all the libs/bins have that lib), then why not adding all the libs to eina and none to the other libs ?
there are several meson.build where some libs are explicitely added.

zmike added a comment.Apr 17 2019, 5:24 AM

Right, but this is a corner case as it only affects the Windows build. In general, linking with meson and modern build toolchains is triggered as-needed, so even if this is added for every library/binary in the tree then it should only end up being functionally included in the places where it is used.

ok, you prefer another approach

but why is this patch wrong ? why don't you want to accept it if it is actually correct ?

bu5hm4n requested changes to this revision.Apr 17 2019, 11:05 PM

It is not *wrong* but it can be improved, and i pretty much second mikes opinion here. Imagine someone will start to use a regex at some point somewhere in the lib. On linux nothing will happen it will just work out of the box. However, it would break *again* windows without the author noticing it.
So this way of doing it is just putting a burdon onto every dev that tries to use regex in the future, while it could be solved so much easier. So PLEASE, just add it as a public dep to evil and everything is good.

This revision now requires changes to proceed.Apr 17 2019, 11:05 PM

it will break if there is a CI for windows...

There is a mingw build which would catch that (which is btw. broken right now with D8644).

However, i pretty much refuse to accept a more complex solution if a other easier is feasible.

it's not my fault if you use an old mingw-w64 which does not have AddClipboardFormatListener (). this function is there since vista (cf https://docs.microsoft.com/sr-latn-rs/windows/desktop/api/winuser/nf-winuser-addclipboardformatlistener ) and has been in minw-w64 for 4 years (see https://sourceforge.net/p/mingw-w64/mailman/message/34033485/ ). It works perfectly on my computer with win7. So what is broken is not D8644, it's your CI for windows.

Its not *my* CI, its the CI of EFL. Undependend from if its the issue of the CI or the issue of the revision. *THIS* revision can be done easier.

so if you don't care i'll abandon this revision and i'll do both fnmatch and regex removal, i'm fed up this takes so long

vtorri abandoned this revision.Apr 18 2019, 7:31 AM