Opened 7 years ago

Closed 6 years ago

#5490 closed Bug (fixed)

daemon won't compile on Windows

Reported by: cfpp2p Owned by: jordan
Priority: Normal Milestone: None Set
Component: libtransmission Version: 2.82+
Severity: Normal Keywords:
Cc:

Description

function chkFilename() is undefined

r13967 & r13968 I think broke it.

see also https://trac.transmissionbt.com/ticket/5252#comment:13

Change History (7)

comment:1 follow-up: Changed 7 years ago by rb07

It does compile under Cygwin, which is in my opinion the best environment for the daemon (it handles foreign characters, non-valid file/path names, and many other similar problems with no change to Transmission's code). Disclaimer: I don't use the daemon under Windows, I use transmission-remote, that's why I know it compiles fine.

Don't take the code I showed on the referred ticked as good code, I've changed that part a couple of times, mostly adding functionality. The real point is: Windows is not supported, even if there is partial support (and 3rd party patches).

comment:2 in reply to: ↑ 1 ; follow-up: Changed 7 years ago by mike.dld

Replying to rb07:

It does compile under Cygwin, which is in my opinion the best environment for the daemon...

I don't think it would compile with Cygwin either out of the box. The issue is that r13967 added a reference to chkFilename function which is implemented in your latest patch (http://sourceforge.net/p/trqtw/patches/15/) but not in official libtransmission, and I'm sure it is not part of some API provided by Cygwin.

Looks like we have two options here:

  1. merge this function from your patch as was done with other changes in aforementioned revisions, or
  2. change the code to eliminate chkFilename calls, so that it reads
    fh1 = CreateFileW (f1nameUTF16, 0, 0, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); 
    fh2 = CreateFileW (f2nameUTF16, 0, 0, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); 
    

Decision is up to core devs, but I can't say I like the way your implementation operates replacing forbidden symbols with look-alike ones; I'd rather replace them with just underscores. I also doubt whether this kind of cleanup should be done at this level and not higher.

comment:3 Changed 7 years ago by rb07

Version 2.82 compiles out of the box on Cygwin:

$ uname -a
CYGWIN_NT-6.1-WOW64 T7400 1.7.25(0.270/5/3) 2013-08-31 20:39 i686 Cygwin

$ transmission-daemon -V
transmission-daemon 2.82 (14156)

Cygwin doesn't include any code guarded by "#ifdef WIN32", or similar (i.e. is the same as building on Unix).

Both of you are right, my code sample on the other ticket shouldn't have made it into the repository, chkFilename() is not defined if compiled for native Windows w/o the Transmission-Qt for Windows patches.

Version 0, edited 7 years ago by rb07 (next)

comment:4 Changed 7 years ago by cfpp2p

responding to rb07

It does compile under Cygwin, which is in my opinion the best environment for the daemon

Yes, you are correct. However -- I needed a patch like your cleanFilename() to handle illegal or foreign characters in a magnet's temporary display name and then it worked fine, but cleanFilename() should be another ticket added for building under Cygwin...

comment:5 in reply to: ↑ 2 Changed 7 years ago by jordan

Replying to mike.dld:

Looks like we have two options here ... Decision is up to core devs

This core dev's preference would be to go with whatever fix you three can find consensus on. Any of the three of you have done more Windows + Transmission work than livings or I have.

comment:6 Changed 7 years ago by mike.dld

Well, the call is removed in my patch for #4160... Whether to add it back or not, and in which form, is to be decided in #4753 and not here.

comment:7 Changed 6 years ago by mike.dld

  • Resolution set to fixed
  • Status changed from new to closed

Current trunk should compile just fine.

Note: See TracTickets for help on using tickets.