Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4727 closed enhancement (fixed)

Porting to Haiku (Patch increasing portability)

Reported by: martinhpedersen Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: haiku porting gcc-3.3 lm libm m tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Hi!

My branch ( haiku-port at github.com/martinhpedersen/Tor ) has two commits fixing some issues that broke compilation on Haiku.

The first fixes -lm being hardcoded in some Makefile.am. On Haiku ( and I guess possibly other platforms) that library lives in libcore.

The second fixes the usage of preprocessor directives inside macro arguments (src/or/main.c) which is unsupported on GCC < 3.3.

These two commits makes tor compile and run under Haiku, and increases portability :)

Child Tickets

Change History (8)

comment:1 Changed 8 years ago by Sebastian

Status: newneeds_review

Thanks for the patches! Please don't be disconcerted at the below feedback, I really like your patch. If you won't have time to do what I suggest, I'll gladly do it!

First off, please change the status of a trac ticket to "needs review" when you have some code. That helps us find things we should look at quickly :)

For the first patch, can you please update it to preserve order of libs? We've had tons of trouble before where on some platforms the order was significant, which led to hard to track down bugs, particularly on windows. I'm unsure if that is the case here, but it might well be, so better safe than sorry :)

As a general note, we try to keep the first line of a commit message pretty short. If you can think of a better msg in 10 seconds, please update it. Otherwise, just keep it this way, it's not that big a deal. Also, if you want, you could add the changes/ files that we like to do (see doc/HACKING for how to do it, or ask on irc).

Thanks again!

comment:2 Changed 8 years ago by Sebastian

Oh, one more thing, also mentioned in doc/HACKING. We generally keep lines under 80 chars, your second patch violates that in main.c. Use "make check-spaces" to track down stuff like this.

comment:3 Changed 8 years ago by martinhpedersen

Thanks for the feedback Sebastian.

I think the branch should be as suggested now, so please review again :)

comment:4 Changed 8 years ago by Sebastian

That looks quite great! I pushed an update to branch haiku-port in my repo (at https://gitweb.torproject.org/sebastian/tor.git/shortlog/refs/heads/haiku-port ), please confirm that this is OK with you and I think we can merge.

comment:5 Changed 8 years ago by martinhpedersen

Looks OK, go ahead :)

comment:6 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-final
Resolution: fixed
Status: needs_reviewclosed

I'm going to merge this, with a few more tweaks:

1: The zen of autoconf says that, to the extend possible, we should always have our tests check for what we actually want to do in our program... and Tor never uses cos(). I'm going to have it test for pow() instead, since we do use that one. (Also log and exp.)

2: Looks like there's trailing whitespace after a backslash in src/test/Makefile.am. I'll fix that.

3: I'm tweaking the error message slightly, since most systems don't even have a "libcore".

4: I'm editing the changes file to explain *why* we need to support gcc 3.3

comment:7 Changed 7 years ago by nickm

Keywords: tor-client added

comment:8 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.