osx-cross / homebrew-avr

Homebrew AVR Toolchain
BSD 2-Clause "Simplified" License
394 stars 79 forks source link

Apply avr-gcc-11-fix-argument-type-mismatch.patch in avr-gcc@11.rb #288

Closed jmechnich closed 1 year ago

jmechnich commented 1 year ago

Apply patch merged in #287.

jmechnich commented 1 year ago

Well, I guess this patch is specific to ARM after all...will move its application into the ARM-specific part of the formula.

jmechnich commented 1 year ago

It looks like the CI run for macos-11 failed due to some external problem. Maybe it just has to be re-run? Also, I was trying to check if there is an option for the test-bot to run on an ARM machine but I could not find anything (it looks as if those tests run on Intel hosts exclusively).

jmechnich commented 1 year ago

I see the same test failing on my local machine, also for e.g. avr-gcc@12. It is caused by variable lookups inside the test section of the formulae, such as

system "#{Formula["avr-binutils"].opt_bin}/avr-objcopy", "-O", "ihex", "-j", ".text", "-j", ".data",  "hello.c.elf", "hello.c.hex"

where a path from another formula is referenced. I have not completely understood how to fix this but it seems to be independent of this PR's content. I am guessing that either this type of reference has to be done differently or that it is in fact a bug in homebrew. The line in Homebrew/api.rb triggering the error has a quite recent change date...

jmechnich commented 1 year ago

Opened issue https://github.com/Homebrew/brew/issues/14752.

ladislas commented 1 year ago

thanks @jmechnich!

a workaround could be to remove the test completely and/or replace it by something trivial.

The test is a bit of a pain as it needs to first compile from source to be able to run the test, have it fail, and then update the expected output. But just from the output it's not possible to know if the compiled binary actually work.

we can test that we can compile a C program and a C++ program, I think that would be enough.

jmechnich commented 1 year ago

The test is a bit of a pain as it needs to first compile from source to be able to run the test, have it fail, and then update the expected output. But just from the output it's not possible to know if the compiled binary actually work.

I don't think I understand what you are saying. Where does the test (normally) fail?

ladislas commented 1 year ago

I'm saying that the current test is not very informative as we don't know if the compiled binary actually works on the target hardware. dumbly comparing the hex to an expected value doesn't make much sense when the compiler changes as we would not know if the difference is good or bad.

to put it in a nutshell, I think the current test can be refactored to just compile something and expect the compilation to succeed.

This should fix the issue as it would allow us to remove the faulty system "#{Formula["avr-binutils"].opt_bin}/avr-objcopy", line

jmechnich commented 1 year ago

290 should fix brew test.

jmechnich commented 1 year ago

@ladislas do you want to rerun the CI for the remaining PRs and merge them before I give this one a final rebase ?

ladislas commented 1 year ago

@jmechnich I'm investigating why the PR failed, it seems to be an issue with Github API limits.

This should fix things: https://github.com/osx-cross/homebrew-avr/pull/300

When it's green and merged, I'll let you rebase your PRs on top of main.

ladislas commented 1 year ago

You can rebase :)

jmechnich commented 1 year ago

@ladislas There is still something funky, it seems...

Not sure what is causing this for acr-gcc@5:

Full audit osx-cross/avr/avr-gcc@5 --online --git --skip-style output
  osx-cross/avr/avr-gcc@5:
    * Formula refers to a GitHub issue or pull request that is closed: https://github.com/riscv/riscv-gnu-toolchain/issues/800
  Error: Formula refers to a GitHub issue or pull request that is closed: https://github.com/riscv/riscv-gnu-toolchain/issues/800
  Error: 1 problem in 1 formula detected

Edit: oh, I see...it is just about the comments https://github.com/osx-cross/homebrew-avr/blob/e15c9e4ee77b9a2d4d071b26d5b300d351ffa308/Formula/avr-gcc%405.rb#L52

ladislas commented 1 year ago

@jmechnich yes, I'm trying to fix that, I'll keep you posted

see https://github.com/osx-cross/homebrew-avr/pull/304

ladislas commented 1 year ago

closed by 0730bda06e8295ae0a5a8d18976b7a3c3b0fbf9b