kostya / lexbor

Fast HTML5 Parser with CSS selectors. This is successor of myhtml and expected to be faster and use less memory.
MIT License
95 stars 14 forks source link

remove makefile dependency #27

Closed etra0 closed 1 year ago

etra0 commented 1 year ago

Since the long goal of Crystal is to be able to be used in Windows as well, I was thinking on porting the make dependency to be simply Crystal scripts.

From what I see, Crystal itself uses Crystal scripts as utils, it is convenient because it's a dependency that will be always satisfied if you well, want to use Crystal.

More tested is needed so that's why I'm doing this as a draft PR, but I'd like to know if this is something you'd be interested as well.

kostya commented 1 year ago

I think this would not work, because you still require cmake, make to build lexbor-c. If you already install cmake, installing make not that big problem, or not?

etra0 commented 1 year ago

Cmake does not has as a prerequisite make, in fact, my system doesn't have make installed and with this I am able to build the static library, because it uses the appropiated build system depending on your OS when we do cmake --build .

Still need to figure out some linking issues I'm having, but the library itself builds now

kostya commented 1 year ago

and i not realy prefer this, because i think, shards command became much slower.

etra0 commented 1 year ago

That's fair, I did miss the option to use 4 threads, but I can understand if you don't want this, I guess I can maintain the fork and upstream it when there's significant changes.

kostya commented 1 year ago

no i mean it can be slower, because crystal compile and run this file (crystal not fast at compiling). is it worked on windows?

etra0 commented 1 year ago

Oh yeah, sadly crystal run is still a bit slower than I'd like, it's my hope that it'll improve, the cold start is a real pain. I guess using python could be also an option, since it has wide adoption, at least I think its adoption is wider than mingw32-make.exe but I don't have any hard statistics to backup this.

is it worked on windows?

I'm still ironing out some linking issues, I'll let you know as soon as I have it working, but the build itself succeeds.

etra0 commented 1 year ago

I have solved the linking issues, but this does comes with a huge tradeoff,

I think lexbor/lexbor when building the static library should use the /MT flag instead of the /MD flag, which means I'd have to do a PR in their repo and see if that's even correct. Then, if said PR get's merged, the REF would have to be updated to a quite recent one, which could come with its own issues.

At this point I'm not sure if I should keep investing time in it or just accept the fact that I won't be able to use this on Windows. FWIW after fixing the linking issues, the library worked as expected.

The other option would be hijack the CMakeCache and change all the /MD to /MT within the script, but I think that would make the script way more extensive than it should be.

etra0 commented 1 year ago

Ok, I think I fixed the linking issues in Windows, it was more easier than I expected, I based my changes from the win.yml from the crystal repo itself.

Here's shards install on my M1 macbook air without my patches (i.e. using make)

shards install  9.33s user 3.38s system 124% cpu 10.175 total

Here's shards install in this branch:

shards install  14.81s user 4.79s system 155% cpu 12.570 total

It's about ~2.4s slower, which is roughly the startup time of crystal run, sadly.

Now this builds and works in Windows though.

kostya commented 1 year ago

i merge modified version of it. it should work from version 3.1.0