search

torrust / torrust-tracker

A modern and feature-rich (private) BitTorrent tracker.
https://torrust.com
GNU Affero General Public License v3.0
364 stars 43 forks source link

Torrust-Tracker won't compile on Windows #325

Open Power2All opened 1 year ago

Power2All commented 1 year ago

Just tried to compile this on Windows CLion with Rust. These modules are not compatible with Windows:

You might want to look into replacing these, or looking to remove them. Unless of course, you won't want to support multi-platforms.

Power2All commented 1 year ago

Found the problem why it won't compile on Windows. You use OpenSSL, which breaks. Would advice to switch to Rustls, which does compile correctly on all platforms.

josecelano commented 1 year ago

Found the problem why it won't compile on Windows. You use OpenSSL, which breaks. Would advice to switch to Rustls, which does compile correctly on all platforms.

Thank you @Power2All. I have to spend some time to understand the problem. Rigth now I only know:

I think we should:

  1. Decide which platforms we want to support.
  2. Make sure the app compiles for all of them.
  3. Add a workflow to check cross-compilation as I mentioned here.

I guess we want to support Windows unless there is a major performance issue or something like that. I think there is no active contributor using Windows for development right now.

cc @da2ce7 @WarmBeer

Power2All commented 1 year ago

We changed https://github.com/torrust/torrust-tracker/issues/68 the openssl dependency to vendored.

I tried this too, but didn't work out as expected, as it's force to use MSVC, which you definitely don't want to use for compiling.

I've fixed all the issues on my end by skipping most of these -sys libraries it tries to use. OpenSSL seems to be the only issue, it does this in combination with ring library, hence the issue.

I think there is no active contributor using Windows for development right now.

Before most of the changes, I was the one using Windows, with CLion as IDE. Since I've been too busy with other projects, I wasn't paying attention on the changes on Torrust-Tracker (I've noticed RwLock is a performance hit, like a lot, compared when using Crossbeam Channel for mutex read and write actions, which I will be trying to work out anyway).

GGLinnk commented 5 months ago

I investigated one of the 5 current test problems.

It seems that reqwest behaves differently under windows because of the way ports are managed. But this only seems to have an impact on testing. The build actually runs without any problems (as far as I've tested). Perhaps the ports are released somewhere in the tests, causing this problem.

Impacted test function: should_assign_to_the_peer_ip_the_remote_client_ip_instead_of_the_peer_address_in_the_request_param at

let status = client.announce(&announce_query).await.unwrap().status() ;

More information coming soon.

GGLinnk commented 5 months ago

I've discovered that, for some unknown reason (yet), on Windows you can't use bind (it works using new).

Edit: It seems that we can't use a LAN IP to the reqwest builder. I mean not the way it is currently made. Maybe there is more things to do to get some kind of permission ?

josecelano commented 5 months ago

I've discovered that, for some unknown reason (yet), on Windows you can't use bind, it works using new.

Edit: It seems that we can't use a LAN IP to the reqwest builder. I mean not the way it is currently made. Maybe there is more things to do to get some kind of permission ?

Hi @GGLinnk, can you paste the code that is not working? I don't know what "bind" or "new" methods you are referring to. Also, what LAN IP is not working? You can also add links to resources, related problems, or whatever you think might be helpful to help with this task.

GGLinnk commented 5 months ago

Ok, for the context, I'm working on this part of the code : https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/v1/contract.rs#L734-L760

This test creates a client using a bind method : https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/v1/contract.rs#L747 It uses two variables:

The bind methods creates a Client (Self) with the following reqwest builder : https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/client.rs#L37

here local_address = client_ip

I've tried using the new method (and so, without the client_ip) and the test completed successfully. For reference the new method is exactly the same just without the .local_address(local_address) https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/client.rs#L28

I'm wondering what is the purpose to manually precise the local address here ? Is that required ? Does the test passing is a good sign? Or does it requires reinforcement ?

josecelano commented 5 months ago

Ok, for the context, I'm working on this part of the code :

https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/v1/contract.rs#L734-L760

This test creates a client using a bind method :

https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/v1/contract.rs#L747

It uses two variables:

  • *env.bind_address(): Returns a locahost address (127.0.0.1) with a random port)
  • client_ip (local_ip().unwrap()): A local network address (mine was 192.168.1.28 for reference).

The bind methods creates a Client (Self) with the following reqwest builder :

https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/client.rs#L37

here local_address = client_ip

I've tried using the new method (and so, without the client_ip) and the test completed successfully. For reference the new method is exactly the same just without the .local_address(local_address)

https://github.com/torrust/torrust-tracker/blob/b92401fcf835984641ad91a1ea88526f4b4cdadc/tests/servers/http/client.rs#L28

I'm wondering what is the purpose to manually precise the local address here ? Is that required ? Does the test passing is a good sign? Or does it requires reinforcement ?

Hi @GGLinnk I don't remember very well that part but I think I bound the local IP because I wanted to add this assert:

assert_eq!(peer_addr.ip(), client_ip);

The only way to know the client IP was to bind to a specific one. If I remember well, the client sometimes used different IPs. Maybe when you run it with docker (E2E tests), you get a different IP than the one you get running it directly.

It seems that in Windows, you cannot bind the client to the port returned using the local_ip function.

/// Retrieves the local IPv4 address of the machine in the local network from
/// the `AF_INET` family.
///
/// A different approach is taken based on the operative system.
///
/// For linux based systems the Netlink socket communication is used to
/// retrieve the local network interface.
///
/// For BSD-based systems the `getifaddrs` approach is taken using `libc`
///
/// For Windows systems Win32's IP Helper is used to gather the Local IP
/// address
pub fn local_ip() -> Result<IpAddr, Error> {

The test passes because the client's IP is the same as the one returned by local_ipbut that does not happen in some cases (probably the one I mentioned, docker).

You can try to run the E2E test in Windows to check if the tests also pass with new instead of bind:

cargo run --bin e2e_tests_runner ./share/default/config/tracker.e2e.container.sqlite3.toml

We could try to find a different way to know which IP the client is using instead of binding to a specific one. Maybe we can make another request before, and the server can return the client's IP, but I would not like to make this test depend on another feature.