cinstall: destdir path joining problems on Windows with absolute paths

[lazka]

I run

cargo cinstall --release --frozen --library-type=cdylib --destdir=C:/foo

but things get installed into C:/usr:

C:/usr/
└── local
    ├── bin
    │   └── rav1e.dll
    ├── include
    │   └── rav1e
    │       └── rav1e.h
    └── lib
        ├── librav1e.dll.a
        ├── pkgconfig
        │   └── rav1e.pc
        └── rav1e.def

[lu-zero]

You should provide more information on which flavor of windows and how you are getting to this. I do not use windows so I'll need your help in tracking it down.

[1480c1]

I think he uses windows with msys2, idk the windows version number

this is probably similar to the situation I mentioned in daala irc

[lazka]

yeah, under MSYS2. I worked it around for now by passing a relative prefix (which results in the wrong prefix in the .pc file, but that is ignored on Windows anyway)

My guess is that when concatenating destdir with an absolute prefix only the prefix survives (destdir="C:/foo", prefix="C:/bar" -> "C:\bar")

[lu-zero]

please try to specify all of them and report back

…

On Tue, Jul 7, 2020, 19:51 Christoph Reiter ***@***.***> wrote: yeah, under MSYS2. I worked it around for now by passing a relative prefix (which results in the wrong prefix in the .pc file, but that is ignored on Windows anyway) My guess is that when concatenating destdir with an absolute prefix only the prefix survives (destdir="C:/foo", prefix="C:/bar" -> "C:\bar") — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#89 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2LJA6RJ2ZG2LH4JZV6RDR2NOBDANCNFSM4OSAQP4Q> .

[lazka]

please try to specify all of them and report back

You mean prefix and destdir? --destdir=C:/foo --prefix=C:/bar -> things are written to C:/bar

I can try to look into it if wanted, though my rust skills are minimal.

[lu-zero]

without c:/

…

On Tue, Jul 7, 2020, 20:00 Christoph Reiter ***@***.***> wrote: please try to specify all of them and report back You mean prefix and destdir? --destdir=C:/foo --prefix=C:/bar -> things are written to C:/bar — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#89 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2LJAZMRHX5FBFR7TFLITR2NPDVANCNFSM4OSAQP4Q> .

[lazka]

--prefix=bar --destdir=foo -> written to ./foo/bar

[lu-zero]

who wants to make a patch to fix destdir joining :) ?

…

On Tue, Jul 7, 2020, 20:05 Christoph Reiter ***@***.***> wrote: --prefix=bar --destdir=foo -> written to ./foo/bar — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#89 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2LJCVTWUUVSA3Y263CZDR2NPVRANCNFSM4OSAQP4Q> .

[1480c1]

I'm assuming it's somewhere around here? https://github.com/lu-zero/cargo-c/blob/master/src/cinstall.rs#L54

[lazka]

my attempt :)

fn append_to_destdir(destdir: &PathBuf, path: &PathBuf) -> PathBuf {
    let mut components = path.components();
    while components.as_path().has_root() {
        components.next();
    }
    destdir.join(components.as_path())
}

[lu-zero]

Ideally you could skip only the Component::Prefix(_) using a patter match. That loop would strip too much

[lu-zero]

I'm assuming it's somewhere around here? https://github.com/lu-zero/cargo-c/blob/master/src/cinstall.rs#L54

That's right, apparently it the case of prefix it should be stripped before stripping the /.

[lazka]

Ideally you could skip only the Component::Prefix(_) using a patter match. That loop would strip too much

You need to remove Prefix (C:) and RootDir (/)

[lu-zero]

if it works the great :) please send a pull request

…

On Tue, Jul 7, 2020, 22:00 Christoph Reiter ***@***.***> wrote: Ideally you could skip only the Component::Prefix(_) using a patter match. That loop would strip too much You need to remove Prefix (C:) and RootDir (/) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#89 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAB2LJDGXBXKQIGPLI2Z5BDR2N5HVANCNFSM4OSAQP4Q> .

[dwbuiten]

It would need to match all drive letters, and not just C:, FWIW