Print

Please login or register.
1 Hour 1 Day 1 Week 1 Month Forever
Login with username, password and session length

[Gaznevada]

Hi all,

compilation of aMule-SVN-r9393 (and previuos versions) on IRIX 6.5, gcc 3.3,
is broken in two points.

-- Point ONE --
First, in Format.cpp, original SVN source code includes "stdint.h" if _MSC_VER
is not defined. _MSC_VER is actually NOT defined on IRIX but the appropriate
include is 'inttypes.h', given that in IRIX systems (maybe others?) stdint.h does not
exist. "configure" clearly and neatly detects proper includes but source code behaves
differently. Note that this conditional "include" is the only difference between the
same file 2.2.3 version, which was working flawlessly. I suspect a broken logic in
the decision of which file to include (if one at all..)

So an ugly hack would be:

*** ./src/libs/common/Format.cpp        Fri Jan 23 14:42:28 2009
--- ./src/libs/common/Format.cpp.orig   Fri Jan 23 14:42:04 2009
***************
*** 27,33 ****
  #include <ctype.h>

  #ifndef _MSC_VER
! #     include <inttypes.h>
  #endif


--- 27,33 ----
  #include <ctype.h>

  #ifndef _MSC_VER
! #     include <stdint.h>
  #endif

I realize is more than ugly but I have no time to make it better...

-- Point TWO --
For a strange reason I couldn't find, "amule.h" needs two brand new includes,
math.h and unistd.h, otherwise a "select" function call in amuled.cpp,
line 308, seems to be undefined, along with a lot of complaints about
various mathematical functions. Don't ask.

A workin' patch would be:

*** ./src/amule.h       Sat Jan 24 21:10:16 2009
--- ./src/amule.h.orig  Sat Jan 24 11:16:49 2009
***************
*** 31,38 ****
  #include <wx/app.h>           // Needed for wxApp
  #include <wx/intl.h>          // Needed for wxLocale

- #include <math.h>
- #include <unistd.h>

  #include "Types.h"            // Needed for int32, uint16 and uint64
  #include <map>
--- 31,36 ----

With those two patches SVN-r9393 compiles flawlessly on my IRIX system.
The very same unpatched version, compiled on a MinGW environment, works
smoothly and the client side, amulegui, properly connects to a working server side
on my remote IRIX machine.

BTW: Windows amulegui client SVN-r9393 connects to a 2.2.3 server. Is this
a change to the rules or what?

Cheers,

- -
Gazneveda

[GonoszTopi]

- #include <math.h>
- #include <unistd.h>

Are you sure you remove those two lines?

Btw, thanks for the testing and the patches.

BTW: Windows amulegui client SVN-r9393 connects to a 2.2.3 server. Is this
a change to the rules or what?

Not one that I know of.

[Gaznevada]

- #include <math.h>
- #include <unistd.h>

Are you sure you remove those two lines?

Btw, thanks for the testing and the patches.

BTW: Windows amulegui client SVN-r9393 connects to a 2.2.3 server. Is this
a change to the rules or what?

Not one that I know of.

Oops.. my typo. Please, swap the files, I add those two lines in patched amule.h

Test and patches: just only my duty of officer and gentleman   

Cheers,

--
Gaznevada

[GonoszTopi]

-- Point TWO --
For a strange reason I couldn't find, "amule.h" needs two brand new includes,
math.h and unistd.h, otherwise a "select" function call in amuled.cpp,
line 308, seems to be undefined, along with a lot of complaints about
various mathematical functions. Don't ask.

Is it only for amuled, or do you get those complaints also for monolithic and remote-gui?

[lfroen]

For a strange reason I couldn't find

Don't ask

Sorry, but I WILL ask. Please find that reason, and it's better be a good one. If you asking to patch the source to fix compilation on discontinued system, which almost nobody using, do the homework first.

[Gaznevada]

For a strange reason I couldn't find

Don't ask

Sorry, but I WILL ask. Please find that reason, and it's better be a good one. If you asking to patch the source to fix compilation on discontinued system, which almost nobody using, do the homework first.

Yes. All in all, you're right. But I have not too much spare time. I'll try to do my best.
But I'd like to point out an issue that *could* be arise in other less discontinued systems.

Cheers,

--
Gaznevada

[Gaznevada]

-- Point TWO --
For a strange reason I couldn't find, "amule.h" needs two brand new includes,
math.h and unistd.h, otherwise a "select" function call in amuled.cpp,
line 308, seems to be undefined, along with a lot of complaints about
various mathematical functions. Don't ask.

Is it only for amuled, or do you get those complaints also for monolithic and remote-gui?

I'll try to do some checks. For the sake of clarity, I get this result when I use the
"--disable-monolithic" option.
Your suggestion is to compile with no "--disable-monolithic" option (and
keeping the original SVN files)? Is this correct?

Just to be sure, given that a full build takes about 24 hours...

Cheers,

--
Gaznevada

[wuischke]

lfroen, if he could give you a reason, (broken xxx in yyy) would it make a difference in your logic? I don't think so, it would just be a waste of his time.

The point is:
* We can make aMule compatible to more systems. (Maybe other unices have the same problem?)
* We don't need ugly ifdefs or anything which is bad. There are no negative effects on other platforms.
* Heck, we even get the solution presented on a silver tablet instead of a "make aMule work on Mac OS (ancient version)". He did his homework.

Gaznevada, you don't have to present the exact reason for your patches. If it works and doesn't break anything else, it does matter very little to us why it happens. (Well, it might be interesting to you.)

Edit: Yes, please omit the --disable-monolithic flag to compile aMule and if you have the time add the flag --enable-amule-gui to test compilation of the remote gui, too.

[Gaznevada]

The point is:
* We can make aMule compatible to more systems. (Maybe other unices have the same problem?)

This is my main concern, really.

you don't have to present the exact reason for your patches. If it works and doesn't break anything else, it does matter very little to us why it happens. (Well, it might be interesting to you.)


Edit: Yes, please omit the --disable-monolithic flag to compile aMule and if you have the time add the flag --enable-amule-gui to test compilation of the remote gui, too.

As already said, it is interesting for me, but I have not this much time...
Anyway, a new compilation has started right now.

BTW: amule-gui launched from the IRIX server (with patched sources) through
a connection with a remote X-server works fine, so it seems that patches does not break anything, up to now...

Cheers,

-- 
Gaznevada

[Kry]

For a strange reason I couldn't find

Don't ask

Sorry, but I WILL ask. Please find that reason, and it's better be a good one. If you asking to patch the source to fix compilation on discontinued system, which almost nobody using, do the homework first.

I think lfroen is a pretty cool guy, eh berates helpers and doesn't afraid of anything.

[lfroen]

lfroen, if he could give you a reason, (broken xxx in yyy) would it make a difference in your logic? I don't think so, it would just be a waste of his time.

The point is:
* We can make aMule compatible to more systems. (Maybe other unices have the same problem?)
* We don't need ugly ifdefs or anything which is bad. There are no negative effects on other platforms.
* Heck, we even get the solution presented on a silver tablet instead of a "make aMule work on Mac OS (ancient version)". He did his homework.

You're fixing something not broken. If you don't know WHY you changing this or that, you very easy get to "broken xxx in yyyy". Did you checked that "there are no negative effects"?! I guess you don't.

[wuischke]

It is broken for Gaznevada and I have no reason not to trust his word. I do test patches before committing and I am responsible for fixing it - or reverting it - if a patch I commit breaks anything. And that's what I do.

BTW: Funny enough, in general I agree to your argument. It's just not feasible to read through IRIX includes to search for an obscure problem.

[Gaznevada]

You're fixing something not broken. If you don't know WHY you changing this or that, you very easy get to "broken xxx in yyyy". Did you checked that "there are no negative effects"?! I guess you don't.

I get the point when you say that it's way much better to know WHY you
commit a patch. Please consider that I'm just a final end-user (this
is not the first time I say so  ) with some exposure on programming,
so I tried to point out an issue which could exist on other systems and
fixing it with a proposed solution working on my system.

But "you're fixing something not broken" is false, at least for me: in
IRIX is broken, and that is a fact. I have not the hybris to pretend
to be a developer and impose my coding choices but:

a) it was broken for me
b) maybe it will be broken for somebody else, who knows?
b) it's not broken anymore

Now it seems aMule-SVN-r9393 is working smoothly with no side effects,
as far as I know and sure, I'd like to know WHY such a patch works but it
takes a time to do that (and sure I'll try to do my best to spot the problem).
Even if I don't know anything about Newton's law,
when  I see a rock falling just above my head, I'd better step aside, not waiting
to know about gravity law. IMHO.

Cheers,

--
Gaznevada

[Kry]

Gaznevada, you did nothing wrong. We welcome your patches. I personally understand the need for the changes in IRIX, as I've worked with IRIX systems in the past. A lot of systems have a different need for includes because we shouldn't rely on some system headers including others (and we do this way too much, including with wx). Your includes are correct for the functions we use, and so it is.

Please don't take this thread as an example of being unwelcome or anything, and thanks again for everything you've done.

[Gaznevada]

Gaznevada, you did nothing wrong. We welcome your patches. I personally understand the need for the changes in IRIX, as I've worked with IRIX systems in the past. A lot of systems have a different need for includes because we shouldn't rely on some system headers including others (and we do this way too much, including with wx). Your includes are correct for the functions we use, and so it is.

Please don't take this thread as an example of being unwelcome or anything, and thanks again for everything you've done.

Kry an all the other guys out there: I definitely don't take this thread as an example of being unwelcome. No need to say that, really.

Mr. Lfroen, albeit a little bit roughly, made a point: you have to know WHY you make a patch. I'm
trying to dig into the guts of IRIX to catch the "why" (e.g.: "select" needs a header not needed by a
GNU/Linux system) but before knowing why, I'd prefer to raise the question proposing a solution.
As previously said, step aside when the rock is falling. Gravity laws later. Indeed, understanding
gravity laws is mandatory.

BTW: I confirm the broken compilation even with"--disable-monolithic" clause. I sent a log
of the compiler to wuischke by e-mail with a proposal of a less intrusive patch.

Cheers,

--
Gaznevada