keyboardio / Kaleidoscope-Bundle-Keyboardio

A Kaleidoscope distribution for the Keyboardio Model 01 and other keyboards.
Other
17 stars 24 forks source link

"Malformed archive" error with joined archive build #16

Closed gedankenexperimenter closed 5 years ago

gedankenexperimenter commented 5 years ago

With #15 it is now possible to get an error from avr-ar when building. I can reproduce this reliably on mac OS with Arduino 1.8.8 (I don't want to risk upgrading to 1.8.9 until someone else can reproduce the problem). This happens when adding or removing an unused class variable in a header file (and probably in other cases).

Here's how I was able to reproduce the problem:

  1. Make a fresh clone of Kaleidoscope-Bundle-Keyboardio
  2. Remove the temporary build dir: rm -rf $TMPDIR/kaleidoscope-$USER
  3. Build the Model01-Firmware sketch
  4. Add a private variable to class Kaleidoscope_ in Kaleidoscope.h: static uint8_t dummy_;
  5. Build Kaleidoscope again

Surprisingly, the second build fails with a "Malformed archive" error from avr-ar:

[merlin@brunelleschi:~/test/keyboardio/avr/libraries/Model01-Firmware] 2019-05-26 04:37
$ BOARD_HARDWARE_PATH=../../../.. make
BOARD_HARDWARE_PATH="../../../.." ../../../../keyboardio/avr/libraries/Kaleidoscope/bin//kaleidoscope-builder build-all
Building ./Model01-Firmware 0.0.0 into /var/folders/5j/1cp97c6d49v5xwhfqsfkn5w80000gn/T//kaleidoscope-merlin/sketch/12908385099-Model01-Firmware.ino/output...
- Size: firmware/Model01-Firmware/Model01-Firmware-0.0.0.elf
  - Program:   26060 bytes (90.9% Full)
  - Data:       1343 bytes (52.5% Full)

[merlin@brunelleschi:~/test/keyboardio/avr/libraries/Model01-Firmware] 2019-05-26 04:38
$ BOARD_HARDWARE_PATH=../../../.. make
BOARD_HARDWARE_PATH="../../../.." ../../../../keyboardio/avr/libraries/Kaleidoscope/bin//kaleidoscope-builder build-all
Building ./Model01-Firmware 0.0.0 into /var/folders/5j/1cp97c6d49v5xwhfqsfkn5w80000gn/T//kaleidoscope-merlin/sketch/12908385099-Model01-Firmware.ino/output...
/var/folders/5j/1cp97c6d49v5xwhfqsfkn5w80000gn/T//kaleidoscope-merlin/ccache/bin//avr-ar: /var/folders/5j/1cp97c6d49v5xwhfqsfkn5w80000gn/T//kaleidoscope-merlin/sketch/12908385099-Model01-Firmware.ino/build/Model01-Firmware.ino_joined.a: Malformed archive
exit status 1
make: *** [build-all] Error 1
noseglasses commented 5 years ago

Thanks @gedankenexperimenter. Now I am able to reproduce the issue.

it did not take me long to find a possible fix which is to simply remove the ..._joined.a archive before every generating a new one. This would be very simple adding another postlink hook in platform.txt.

Unfortunately I did not yet find a way to accomplish this in a portable way. i spend the whole afternoon trying this and that but without success.

Using rm in platform.txt is simply not an option as non-portable. The avr-ar command on the other hand has no flag that would help us. it does not support a forced re-creation of an archive.

I even considered using java as it comes with Arduino. But i don't think that's a good idea.

@obra, do you have any idea how to portably erase a file from a command specified in platform.txt?

If not, I recommend to revert the change to re-enable safe rebuilds (without malformed archives).

gedankenexperimenter commented 5 years ago

Would it be practical to have kaleidoscope-builder remove the old joined archive before calling arduino-builder?

gedankenexperimenter commented 5 years ago

…or do builds from the Arduino GUI not use kaleidoscope-builder?

noseglasses commented 5 years ago

Would it be practical to have kaleidoscope-builder remove the old joined archive before calling arduino-builder?

kaleidoscope-builder is a bash script and thus not available under Windows (except for mingw). AFAIK it's special purpose for Kaleidoscope. That's why the Arduino GUI does not use it.

To stay portable, platform.txt can only rely on executables that come with Arduino such as the avr-... versions of binutils and possibly java.

Up to now i did not find any suitable way to abuse one of binutils tools to erase a file. That's really insane, considering how simple a task that is compared to all the other complex stuff that binutils is designed for.

obra commented 5 years ago

Hm. Would using ar in thin mode fix this? Or using ar's "d" facility to rip all the symbols out of the archive? (I'm not skilled at this part of the world. Just throwing out ideas.)

On Sun, May 26, 2019 at 10:17 PM noseglasses notifications@github.com wrote:

Would it be practical to have kaleidoscope-builder remove the old joined archive before calling arduino-builder?

kaleidoscope-builder is a bash script and thus not available under Windows (except for mingw). AFAIK it's special purpose for Kaleidoscope. That's why the Arduino GUI does not use it.

To stay portable, platform.txt can only rely on executables that come with Arduino such as the avr-... versions of binutils and possibly java.

Up to now i did not find any suitable way to abuse one of binutils tools to erase a file. That's really insane, considering how simple a task that is compared to all the other complex stuff that binutils is designed for.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keyboardio/Kaleidoscope-Bundle-Keyboardio/issues/16?email_source=notifications&email_token=AAALC2E75ZEVVPCUNWEOLO3PXKLZHA5CNFSM4HPWJYH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWIGY5Q#issuecomment-496004214, or mute the thread https://github.com/notifications/unsubscribe-auth/AAALC2CSVOJEJ23DFSVWFELPXKLZHANCNFSM4HPWJYHQ .

noseglasses commented 5 years ago

We already use thin mode.

Nice idea. But I already tried the "d" facility to remove all objects from the joined archive after linking the elf. The problem is that one needs to explicitly specify the names of all the objects to delete. But that information is no more available in a postlink hook (Arduino could really be improved here). There is no delete all available.

obra commented 5 years ago

@noseglasses - feel free to push a temporary reversion of the PR to master. ᐧ

On Sun, May 26, 2019 at 10:46 PM Jesse Vincent jesse@keyboard.io wrote:

Hm. Would using ar in thin mode fix this? Or using ar's "d" facility to rip all the symbols out of the archive? (I'm not skilled at this part of the world. Just throwing out ideas.)

On Sun, May 26, 2019 at 10:17 PM noseglasses notifications@github.com wrote:

Would it be practical to have kaleidoscope-builder remove the old joined archive before calling arduino-builder?

kaleidoscope-builder is a bash script and thus not available under Windows (except for mingw). AFAIK it's special purpose for Kaleidoscope. That's why the Arduino GUI does not use it.

To stay portable, platform.txt can only rely on executables that come with Arduino such as the avr-... versions of binutils and possibly java.

Up to now i did not find any suitable way to abuse one of binutils tools to erase a file. That's really insane, considering how simple a task that is compared to all the other complex stuff that binutils is designed for.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keyboardio/Kaleidoscope-Bundle-Keyboardio/issues/16?email_source=notifications&email_token=AAALC2E75ZEVVPCUNWEOLO3PXKLZHA5CNFSM4HPWJYH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWIGY5Q#issuecomment-496004214, or mute the thread https://github.com/notifications/unsubscribe-auth/AAALC2CSVOJEJ23DFSVWFELPXKLZHANCNFSM4HPWJYHQ .

obra commented 5 years ago

Will ar's "t" mode list the members in a format "d" can use them? I'm not sure you can chain commands in platform.txt rules, though. ᐧ

On Sun, May 26, 2019 at 10:50 PM noseglasses notifications@github.com wrote:

We already use thin mode.

Nice idea. But I already tried the "d" facility to remove all objects from the joined archive after linking the elf. The problem is that one needs to explicitly specify the names of all the objects to delete. But that information is no more available in a postlink hook (Arduino could really be improved here). There is no delete all available.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keyboardio/Kaleidoscope-Bundle-Keyboardio/issues/16?email_source=notifications&email_token=AAALC2GNCG2JUO2KMPBAYXTPXKPVDA5CNFSM4HPWJYH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWIHKZY#issuecomment-496006503, or mute the thread https://github.com/notifications/unsubscribe-auth/AAALC2AYZ5BFSPBAXUEH7B3PXKPVDANCNFSM4HPWJYHQ .

gedankenexperimenter commented 5 years ago

I don't know what specific conditions trigger this problem (just the one that I've found). Is it possible to trigger it with changes only in the sketch files? If not, maybe it's better to work around it with a fix for command-line builds only while we try to find a more universal solution. Arduino GUI users are, I expect, considerably less likely than CLI users to be triggering this problem, and the workaround is much more straightforward for this issue than it is for the linker errors fixed by #15.

noseglasses commented 5 years ago

@obra, i reverted master thereby preserving your travis fix.

Chaining commands would really help a lot. But in Arduino's docs I did not find anything about it.

noseglasses commented 5 years ago

Arduino GUI users are, I expect, considerably less likely than CLI users to be triggering this problem, and the workaround is much more straightforward for this issue than it is for the linker errors fixed by #15.

I expect the problem to show up regardless of whether its a command line or GUI build. It's just the way that ar creates its archives under the hood. I didn't ever touch the GUI but i am pretty sure that your approach to force the issue by adding the additional member will also break the GUI build.

gedankenexperimenter commented 5 years ago

What I meant was that users who build firmware with the GUI are much less likely to make changes that could trigger the problem.

noseglasses commented 5 years ago

What I meant was that users who build firmware with the GUI are much less likely to make changes that could trigger the problem.

I see what you mean and I agree. But now as you found out what causes the problem, I can imagine many ways how even an unexperienced user could trigger this issue, e.g. by compiling, then updating any of the repos, followed by a rebuild. I am pretty sure that the error will show with any change to any class' inventory. Unexperienced users are usually the ones who are scared off most easily.

algernon commented 5 years ago

With #17 merged, this should be marked resolved, right?

noseglasses commented 5 years ago

Right.