New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CIs #194

Merged
merged 2 commits into from Apr 24, 2018

Conversation

Projects
None yet
2 participants
@GuillaumeGomez
Member

GuillaumeGomez commented Oct 25, 2017

Show outdated Hide outdated .travis.yml
Show outdated Hide outdated appveyor.yml
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 22, 2018

Member

This failure is really weird...

Member

GuillaumeGomez commented Apr 22, 2018

This failure is really weird...

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 22, 2018

Member

Seems there no https://github.com/gtk-rs/gio/blob/master/src/unix_socket_address.rs#L6-L7 for windows.
Currently I don't have access to docs so cant check, have windows OsStrExt or not

Member

EPashkin commented Apr 22, 2018

Seems there no https://github.com/gtk-rs/gio/blob/master/src/unix_socket_address.rs#L6-L7 for windows.
Currently I don't have access to docs so cant check, have windows OsStrExt or not

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 22, 2018

Member

The thing is that I have added this check on gio and it works fine... (As you can see here.)

Member

GuillaumeGomez commented Apr 22, 2018

The thing is that I have added this check on gio and it works fine... (As you can see here.)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 22, 2018

Member

Understand, it really strange.
Gio documented for me locally, but I can't check gdk due bad access to crates.

Member

EPashkin commented Apr 22, 2018

Understand, it really strange.
Gio documented for me locally, but I can't check gdk due bad access to crates.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 22, 2018

Member

I'll try to figure this out later.

Member

GuillaumeGomez commented Apr 22, 2018

I'll try to figure this out later.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 24, 2018

Member

@GuillaumeGomez
Problem that OsExt::from_bytes is unix only.
This patch in gio fixes problem, but it sure wrong way to fix.
Also I still don't understand why docs builds for gio.

--- a/src/unix_socket_address.rs
+++ b/src/unix_socket_address.rs
@@ -78,7 +78,10 @@ impl<O: IsA<UnixSocketAddress> + IsA<glib::object::Object>> UnixSocketAddressExt
         };
         match self.get_address_type() {
             UnixSocketAddressType::Anonymous => Some(Anonymous),
+            #[cfg(unix)]
             UnixSocketAddressType::Path => Some(Path(path::Path::new(OsStr::from_bytes(path)))),
+            #[cfg(not(unix))]
+            UnixSocketAddressType::Path => panic!("Unsupported UnixSocketAddressType::Path"),
             UnixSocketAddressType::Abstract => Some(Abstract(path)),
             UnixSocketAddressType::AbstractPadded => Some(AbstractPadded(path)),
             UnixSocketAddressType::Invalid | UnixSocketAddressType::__Unknown(_) => None,
Member

EPashkin commented Apr 24, 2018

@GuillaumeGomez
Problem that OsExt::from_bytes is unix only.
This patch in gio fixes problem, but it sure wrong way to fix.
Also I still don't understand why docs builds for gio.

--- a/src/unix_socket_address.rs
+++ b/src/unix_socket_address.rs
@@ -78,7 +78,10 @@ impl<O: IsA<UnixSocketAddress> + IsA<glib::object::Object>> UnixSocketAddressExt
         };
         match self.get_address_type() {
             UnixSocketAddressType::Anonymous => Some(Anonymous),
+            #[cfg(unix)]
             UnixSocketAddressType::Path => Some(Path(path::Path::new(OsStr::from_bytes(path)))),
+            #[cfg(not(unix))]
+            UnixSocketAddressType::Path => panic!("Unsupported UnixSocketAddressType::Path"),
             UnixSocketAddressType::Abstract => Some(Abstract(path)),
             UnixSocketAddressType::AbstractPadded => Some(AbstractPadded(path)),
             UnixSocketAddressType::Invalid | UnixSocketAddressType::__Unknown(_) => None,
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 24, 2018

Member

It's done recursively by cargo doc I assume. And yes, not a very beautiful way to do it. :)

Member

GuillaumeGomez commented Apr 24, 2018

It's done recursively by cargo doc I assume. And yes, not a very beautiful way to do it. :)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 24, 2018

Member

As alternative we can add trait and impl for dox:

#[cfg(all(not(unix), feature = "dox"))]
trait OsStrExt {
    fn from_bytes(slice: &[u8]) -> &Self;
}

#[cfg(all(not(unix), feature = "dox"))]
impl OsStrExt for OsStr {
    fn from_bytes(_slice: &[u8]) -> &OsStr {
        unreachable!()
    }
}
Member

EPashkin commented Apr 24, 2018

As alternative we can add trait and impl for dox:

#[cfg(all(not(unix), feature = "dox"))]
trait OsStrExt {
    fn from_bytes(slice: &[u8]) -> &Self;
}

#[cfg(all(not(unix), feature = "dox"))]
impl OsStrExt for OsStr {
    fn from_bytes(_slice: &[u8]) -> &OsStr {
        unreachable!()
    }
}
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 24, 2018

Member

Currently I prefer slightly changed first variant, it IMHO make intentions clearer

#[cfg(not(dox))]
UnixSocketAddressType::Path => Some(Path(path::Path::new(OsStr::from_bytes(path)))),
#[cfg(dox)]
UnixSocketAddressType::Path => unreachable!(),
Member

EPashkin commented Apr 24, 2018

Currently I prefer slightly changed first variant, it IMHO make intentions clearer

#[cfg(not(dox))]
UnixSocketAddressType::Path => Some(Path(path::Path::new(OsStr::from_bytes(path)))),
#[cfg(dox)]
UnixSocketAddressType::Path => unreachable!(),
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 24, 2018

Member

I also do so let's go for it!

Member

GuillaumeGomez commented Apr 24, 2018

I also do so let's go for it!

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin
Member

EPashkin commented Apr 24, 2018

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 24, 2018

Member

Same error. It seems that appveyor is keeping some kind of cache? Well anyway, I'll merge this PR and we'll see...

Member

GuillaumeGomez commented Apr 24, 2018

Same error. It seems that appveyor is keeping some kind of cache? Well anyway, I'll merge this PR and we'll see...

@GuillaumeGomez GuillaumeGomez merged commit de21c06 into gtk-rs:master Apr 24, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:up-cis branch Apr 24, 2018

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Apr 24, 2018

Member

I just written wrong cfg :(

Member

EPashkin commented Apr 24, 2018

I just written wrong cfg :(

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Apr 24, 2018

Member

Wait what? Damn... I badly reviewed then, sorry! ><

Member

GuillaumeGomez commented Apr 24, 2018

Wait what? Damn... I badly reviewed then, sorry! ><

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment