Re: [Spice-devel] [PATCH spice-gtk v2 0/4]Add more tests for the session's uri

2016-10-14 Thread Victor Toso
Hi,

On Wed, Oct 12, 2016 at 05:08:22PM +0200, Pavel Grunt wrote:
> Hi,
>
> I plan to merge both uri parsers (one is in spice-session, one in spice-uri).
> These patches add more to tests to avoid regressions.
>
> v2 per Victor review splits IPv6 tests and checks for expected warnings
> v1: https://lists.freedesktop.org/archives/spice-devel/2016-May/029261.html

Acked-by: Victor Toso 

Regarding Rust, I'll need play a bit with the language to give a proper
opinion.

>
> Thanks,
> 
> Pavel Grunt (4):
>   test-session: Test alternative way for setting port
>   test-session: Also test hostname, username and password
>   test-session: Test invalid URIs
>   test-session: Add IPv6 tests
> 
>  tests/session.c | 266 
> +++-
>  1 file changed, 242 insertions(+), 24 deletions(-)
> 
> -- 
> 2.10.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk v2 0/4]Add more tests for the session's uri

2016-10-13 Thread Marc-André Lureau
Hi

- Original Message -
> Hi,
> 
> On Wed, 2016-10-12 at 11:38 -0400, Marc-André Lureau wrote:
> > Hi
> > 
> > - Original Message -
> > > Hi,
> > > 
> > > I plan to merge both uri parsers (one is in spice-session, one in
> > > spice-uri).
> > > These patches add more to tests to avoid regressions.
> > > 
> > > v2 per Victor review splits IPv6 tests and checks for expected
> > > warnings
> > > v1: https://lists.freedesktop.org/archives/spice-devel/2016-May/02
> > > 9261.html
> > 
> > Reminds me of https://lists.freedesktop.org/archives/spice-devel/201
> > 5-March/019288.html
> > 
> > It's a shame to see glib so slow at picking the GUri patches. I
> > personally lost interest, there are other better ways imho.
> Yes it is
> 
> > 
> > Tbh, instead of slowly reinventing YA URI parsing in C,
> 
> The parser is already implemented in spice-gtk (twice). My intention
> is to have just one parser in spice-gtk, after that it should be
> easier to migrate another parser, fix bugs etc...

Sure, but merging the two will probably result in more complex URI (and yet 
incomplete or incorrect) parsing.

> 
> >  I am tempted to go a step in the future and just link with the rust
> > URI crate (https://docs.rs/url/1.2.1/url/). Since it's already being
> > used by firefox in the latest release, I trust this is fairly solid.
> 
> Probably it is and rust is interesting, but imho it is overkill to use
> rust just to have better/safer url parsing :)

Well, it would have to start somewhere. That's what firefox did (along with mp4 
parsing iirc). I would be really glad to use a safer language with a very 
active community (contrast to glib development, stable or moribound depending 
on pov)

> > I have done some mix of rust and C in the past, just like what Hub
> > recently described in a blog: https://www.figuiere.net/hub/blog/?201
> > 6/10/07/862-rust-and-automake
> > 
> > It is fairly easy, and I would be happy to link with rust code which
> > is way safer than C.
> > 
> > Of course, we would have to keep C for a while, but given that
> > firefox depends on rust and Fedora ships it, gdb support it etc,
> > there is a good chance it ends in every single platform we care
> > about.
> 
> I'm just curios - do you know if it is currently possible to compile
> it using mingw ?

Good point, I haven't looked much at that either. I know the cross-compilation 
story is fairly good, and you can see answer on SO about cross-windows build 
http://stackoverflow.com/questions/31492799/cross-compile-a-rust-application-from-linux-to-windows
 

> Anyway I believe that is better to have tests before we change
> something :)

sure, I would just prefer we reuse libraries when possible instead of 
reinventing the wheel, and URI parsing is a good candidate.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk v2 0/4]Add more tests for the session's uri

2016-10-13 Thread Pavel Grunt
Hi,

On Wed, 2016-10-12 at 11:38 -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > Hi,
> > 
> > I plan to merge both uri parsers (one is in spice-session, one in
> > spice-uri).
> > These patches add more to tests to avoid regressions.
> > 
> > v2 per Victor review splits IPv6 tests and checks for expected
> > warnings
> > v1: https://lists.freedesktop.org/archives/spice-devel/2016-May/02
> > 9261.html
> 
> Reminds me of https://lists.freedesktop.org/archives/spice-devel/201
> 5-March/019288.html
> 
> It's a shame to see glib so slow at picking the GUri patches. I
> personally lost interest, there are other better ways imho.
Yes it is

> 
> Tbh, instead of slowly reinventing YA URI parsing in C,

The parser is already implemented in spice-gtk (twice). My intention
is to have just one parser in spice-gtk, after that it should be
easier to migrate another parser, fix bugs etc...

>  I am tempted to go a step in the future and just link with the rust
> URI crate (https://docs.rs/url/1.2.1/url/). Since it's already being
> used by firefox in the latest release, I trust this is fairly solid.

Probably it is and rust is interesting, but imho it is overkill to use
rust just to have better/safer url parsing :)

> 
> I have done some mix of rust and C in the past, just like what Hub
> recently described in a blog: https://www.figuiere.net/hub/blog/?201
> 6/10/07/862-rust-and-automake
> 
> It is fairly easy, and I would be happy to link with rust code which
> is way safer than C.
> 
> Of course, we would have to keep C for a while, but given that
> firefox depends on rust and Fedora ships it, gdb support it etc,
> there is a good chance it ends in every single platform we care
> about.

I'm just curios - do you know if it is currently possible to compile
it using mingw ?

Anyway I believe that is better to have tests before we change
something :)

Pavel

> 
> > 
> > Thanks,
> > 
> > Pavel Grunt (4):
> >   test-session: Test alternative way for setting port
> >   test-session: Also test hostname, username and password
> >   test-session: Test invalid URIs
> >   test-session: Add IPv6 tests
> > 
> >  tests/session.c | 266
> >  +++-
> >  1 file changed, 242 insertions(+), 24 deletions(-)
> > 
> > --
> > 2.10.1
> > 
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk v2 0/4]Add more tests for the session's uri

2016-10-12 Thread Marc-André Lureau
Hi

- Original Message -
> Hi,
> 
> I plan to merge both uri parsers (one is in spice-session, one in spice-uri).
> These patches add more to tests to avoid regressions.
> 
> v2 per Victor review splits IPv6 tests and checks for expected warnings
> v1: https://lists.freedesktop.org/archives/spice-devel/2016-May/029261.html

Reminds me of 
https://lists.freedesktop.org/archives/spice-devel/2015-March/019288.html

It's a shame to see glib so slow at picking the GUri patches. I personally lost 
interest, there are other better ways imho.

Tbh, instead of slowly reinventing YA URI parsing in C, I am tempted to go a 
step in the future and just link with the rust URI crate 
(https://docs.rs/url/1.2.1/url/). Since it's already being used by firefox in 
the latest release, I trust this is fairly solid.

I have done some mix of rust and C in the past, just like what Hub recently 
described in a blog: 
https://www.figuiere.net/hub/blog/?2016/10/07/862-rust-and-automake

It is fairly easy, and I would be happy to link with rust code which is way 
safer than C.

Of course, we would have to keep C for a while, but given that firefox depends 
on rust and Fedora ships it, gdb support it etc, there is a good chance it ends 
in every single platform we care about. 

> 
> Thanks,
> 
> Pavel Grunt (4):
>   test-session: Test alternative way for setting port
>   test-session: Also test hostname, username and password
>   test-session: Test invalid URIs
>   test-session: Add IPv6 tests
> 
>  tests/session.c | 266
>  +++-
>  1 file changed, 242 insertions(+), 24 deletions(-)
> 
> --
> 2.10.1
> 
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [PATCH spice-gtk v2 0/4]Add more tests for the session's uri

2016-10-12 Thread Marc-André Lureau


- Original Message -
> Hi
> 
> - Original Message -
> > Hi,
> > 
> > I plan to merge both uri parsers (one is in spice-session, one in
> > spice-uri).
> > These patches add more to tests to avoid regressions.
> > 
> > v2 per Victor review splits IPv6 tests and checks for expected warnings
> > v1: https://lists.freedesktop.org/archives/spice-devel/2016-May/029261.html
> 
> Reminds me of
> https://lists.freedesktop.org/archives/spice-devel/2015-March/019288.html
> 
> It's a shame to see glib so slow at picking the GUri patches. I personally
> lost interest, there are other better ways imho.
> 
> Tbh, instead of slowly reinventing YA URI parsing in C, I am tempted to go a
> step in the future and just link with the rust URI crate
> (https://docs.rs/url/1.2.1/url/). Since it's already being used by firefox
> in the latest release, I trust this is fairly solid.
> 
> I have done some mix of rust and C in the past, just like what Hub recently
> described in a blog:
> https://www.figuiere.net/hub/blog/?2016/10/07/862-rust-and-automake
> 

To be fair, I did a bit more work to import/export C symbols and link with the 
static lib. And I didn't solve the out-of-tree/distcheck build issue either.

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel