ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.69k stars 17.16k forks source link

Compiling with cygwin broken!? #27867

Closed olliw42 closed 1 week ago

olliw42 commented 3 weeks ago

I'm trying to compile current master, and get a link error, the culprit seems to be __wrap__malloc_r:

grafik

When I did last time, like 2-3 months back it was working all fine. This stuff really is above my paygrade, but it seems to me it is related to the changes in https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_Common/c%2B%2B.cpp, which happened in these last months. When I ensure that the code section in #ifdef CYGWIN_BUILD is not included, when it compiles and links fine ... but I sure have no idea if the resulting firmware would be working fine.

I'm on Win10 and for compiling I use something which calls itself "Cygwin64 Terminal". I must admit I don't recall really what this is (well, google leads to a page, but ...), just that I use it since long and it always worked great for me so far.

Any hint on what I should do in the code would be much appreciated :)

olliw42 commented 3 weeks ago

maybe these parts of the printout are of interest too

grafik grafik

yuri-rage commented 3 weeks ago

IMHO, we should deprecate use of Cygwin entirely. WSL2 is a far better solution. Recommend using it.

I know that's not a "fix" for your current environment, but it is a much cleaner way of doing things overall.

rmackay9 commented 3 weeks ago

I agree that using WSL2 is a better solution than using cygwin. The only downside is it is more difficult to access USB devices from WSL2 but that is also do-able using USB WSL and/or USB WSL Gui

olliw42 commented 3 weeks ago

why then not a code change of the kind

#ifdef CYGWIN_BUILD
#error Cygwin is no longer supported.
#endif

instead of actual changes ... it's a bit of an obscure way for that purpose I would say ;)

my feeling though is that it only can be a minor code change which would correct the situation ...

robustini commented 3 weeks ago

WSL2 is a far better solution but does not allow compilation of the SITL executable for Windows. So either a solution is found or Cygwin is still required. @olliw42 I don't see a problem with Cygwin, try updating it or start from scratch.

immagine

olliw42 commented 3 weeks ago

MANY THX that's weired ... it's really just the changes I've mentioned

is this the same? I note that my count goes to [1200] while yours go to [1335] ... your's seemt o be for SITL ?!?

could you retry for the board MatekH743?

robustini commented 2 weeks ago

MANY THX that's weired ... it's really just the changes I've mentioned

is this the same? I note that my count goes to [1200] while yours go to [1335] ... your's seemt o be for SITL ?!?

could you retry for the board MatekH743?

Same issue, damn.

olliw42 commented 2 weeks ago

I think that's why it wasn't spotted, because was just used for sitl ... THX for confirming it however, much appreciated

robustini commented 2 weeks ago

@olliw42 Is a memory allocation problem, I believe there has already been an attempt to fix this problem, apparently unsuccessfully, ping @tridge who did hoping for a solution. Cygwin remains convenient in some contexts, I would be sorry if it were deprecated. At the moment to locally compile the executable for SITL in Windows remains critical and fortunately it works. I too always use WSL2 to compile firmware for boards.

https://github.com/ArduPilot/ardupilot/commit/039367e340884d46a4e4a7c16f4e88b0421bec5b

olliw42 commented 2 weeks ago

THX. pinged him on discord

rmackay9 commented 2 weeks ago

I've added this to the 4.5 issues list so it should be discussed at tomorrow's call

tridge commented 2 weeks ago

@olliw42 we don't actually support cygwin as a cross-compiler for ChibiOS boards, we only support it for SITL builds. If you wanted to support that then we would need a CI test for it, but I don't see why it is needed, much better to use WSL2

tridge commented 2 weeks ago

if there is a good reason to support it then we just need to change the #ifdef CYGWIN_BUILD in c++.cpp to check for a SITL build ideally we would also use the same compiler version in cygwin so we get the same binary

tridge commented 2 weeks ago

we discussed this on the dev call, and we think we should fix it with the small ifdef change, along with:

peterbarker commented 2 weeks ago

@olliw42 there's some suggestion we should stop supporting cross-compiling under cygwin.

Most developers are happily using wsl2 now-adays.

Do you have thoughts on the matter?

rmackay9 commented 2 weeks ago

A small correction on Tridge's comment above, we have supported cross-compiling for ChibiOS for a long time (I used it for many years) but it was recently broken due to the malloc changes. Tridge thinks it's a one or two line fix to get this working again.

As PeterB says though, all the other developers on the call using windows have moved to WSL2

I think we should fix the issue for now but look towards dropping support for cygwin except for building SITL. I think with a small team we just need to be careful not to overextend ourselves by supporting cygwin when WSL2 is the recommended method to compile on Windows.

olliw42 commented 2 weeks ago

MANY thx for looking that carefully into this.

I sure have much sympathy for that you folks don't want to maintain all sorts of build systems. It just wasn't that apparent that you might intend to deprecate cygwin for above purpose, i.e. if that's a bug or intention. I sure might have missed the announcement however.

What I don't quite understand in all the discussion however - and that's certainly due to my lack of knowledge and naivity in that area, so sorry for this - is that the output from waf makes me believe that my setup actually doesn't use a cygwin compiler, but just the ordinary gnu arm embeeded arm-none-eabi compiler used by anyone else too ... at least in the printouts I've shared in the 1st and 2nd posts I see gnu arm embeeded and arm-none-eabi all over the place ... if that would be a correct interpretation I would think the issue is not a cygwin crosscompile issue but that somehow the build system mistakenly thinks it's a cygwin compiler and not a gnu compiler, in which case I would think it's something one would want to correct anyway.

olliw42 commented 2 weeks ago

ok, so here my verdict. In the last days I fooled around with (cross-) compiling on Win using WSL.

  1. I can see why most prefer using WSL. I did some compile time tests, and WSL2 wins dramatically time-wise - if the files are stored in the wsl/Linux area. If not, it is terrible. In case it might be of interest, that's my table
cygwin64 WSL 1 WSL 2
ext 6:02 5:10 > 15 min
int - 2:51 0:55

ext means files are on C drive, int means files are on Linux/wsl "drive", times are in minutes, for ext WSL2 I did not let it finish since it was soo f...ing slow

  1. Using the files on the C drive is suggested as a possibility in the docs, and for folks like me it's really the only reasonable option. HOWEVER: IT DOES NOT WORK with the current ardupuilot repository !

The issue is that some relevant files are NOT in Linux format (end of line) but in Win/dos format! This creates a good number of errors. Specifically the files which come in Win format and need to be changed to Linux format are

Strange enough some other relevant files such as Tools/environment_install/install-prereqs-ubuntu.sh and most it calls are stored in Linux format. Now it would be easy to argue that's something stupid I did, so I double checked by freshly cloing the ardupilot repository, and it comes out exactly as I'm saying. I thus think that's some odditiy of the ardupilot repo, and not my fault.

  1. I thus would ask to do these two things:
    • get all line endings in the script files correct. I guess it means all linux script files in Tools should be Linux format.
    • add a comment to the docs saying that if the files are on the win drive but not the wsl drive one should prefer WSL1 since WSL2 will be unusably slow (that's actually a tip I got from INAV's dev build page, thx to INAV)
rmackay9 commented 2 weeks ago

@olliw42,

Thanks so much for the analysis and feedback. In particular it's nice to see the compile time numbers which are in line with what Tridge thougt. Surely we can make the corrections you suggest to the scripts and wiki.

tridge commented 1 week ago

@olliw42 please test this PR: https://github.com/ArduPilot/ardupilot/pull/27987

olliw42 commented 1 week ago

many thx response to test in that PR

any plan to straighten the file line endings too?

tridge commented 1 week ago

fixed by #27987

olliw42 commented 1 week ago

fixed by #27987

unfortunately not I don't understand why one would think that a change in the c code would resolve the issues of the install scripts with the line endings.

running Tools/environment_install/install-prereqs-ubuntu.sh -y note the $'r'$ issues as a result only two path entries are in .profile instead of 3 dito the next steps don't work as they should nothing is solved as regards wsl install

grafik grafik

olliw42 commented 1 week ago

when I change the lin eendings of mentioned fils manually, then the results look as this (markedly different)

grafik

grafik

rmackay9 commented 1 week ago

Hi @olliw42, I'm going to raise a PR to fix the line endings shortly. anyone could do that of course but I'm happy to in any case

olliw42 commented 1 week ago

I think this would be very helpfull I unfortunately have no clue how one would fix that, sorry for not being helpful in terms of PRs here.

robustini commented 1 week ago

Tried compiling for SpeedyBeeF405WING board with PR and it works for me, although it crashes both declaring the board and at the end of compilation and I am forced to give a Ctrl-C and it show me "Error in atexit._run_exitfuncs:". The procedure is successful but there is this annoying block.

It completes the compilation correctly but remains frozen at this point. immagine

After a Ctrl-C. immagine