Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add real gcc ver chk #2118

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Conversation

zxkmm
Copy link
Contributor

@zxkmm zxkmm commented Apr 22, 2024

it's because it happend on me and gull that the current check gcc ver isn't that reliable.
This is read from elf target which were compiled, which should be accurate

NOTE that i'm not sure if this will break pipeline. i need to check on my fork HOWEVER even after it passed from my side it's still need a nightly release to check work

readelf cmd is from binutils. which idk if it's included in pipeline. (edit: pipeline compile passed)

https://github.com/zxkmm/mayhem-firmware/actions/runs/8778357213

@zxkmm zxkmm marked this pull request as ready for review April 22, 2024 04:54
@u-foka
Copy link
Contributor

u-foka commented Apr 24, 2024

Doh, sry totally forgot about this :/

This is nice to be able to tell about any binary what exact version it was built with :)

To fix the detection issue in cmake however it is potentially enough to replace "arm-none-eabi-g++ -v" with "${CMAKE_CXX_COMPILER} -v" (havent tested this yet :))

Will send that as a separate PR, this I'll try to read in the evening it should go in regardless 👍

@Brumi-2021
Copy link
Contributor

Hi ,
I think that @NotherNgineer , already merged another PR that writes in the user settings the used gcc version to create that binary .
Is that the same issue 🤔?

@zxkmm
Copy link
Contributor Author

zxkmm commented Apr 24, 2024

Hi ,
I think that @NotherNgineer , already merged another PR that writes in the user settings the used gcc version to create that binary .
Is that the same issue 🤔?

No. It’s somehow not accurate.

Copy link
Member

@gullradriel gullradriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not.

@gullradriel gullradriel merged commit f572b00 into portapack-mayhem:next Apr 29, 2024
3 checks passed
@NotherNgineer
Copy link
Contributor

It would be better if there was a single message versus one per ELF file -- because now the percentage ROM utilization scrolls off the screen.

@zxkmm
Copy link
Contributor Author

zxkmm commented Apr 30, 2024

It would be better if there was a single message versus one per ELF file -- because now the percentage ROM utilization scrolls off the screen.

Thanks! I'll submit with a new PR with compare.

@zxkmm
Copy link
Contributor Author

zxkmm commented Apr 30, 2024

After thinking, I think it's not quite make sense to compress the output, since it could be possible if ccache mistakenly skipped compiling in some edge case that we didn't know, and it's would be harder to debug.

and about flushing the screen, I think other things do that as well, for example cmake itself:

[ 64%] Built target baseband_gps_sim.elf
[ 66%] Built target baseband_test.elf
[ 70%] Built target baseband_audio_beep.elf
[ 70%] Built target baseband_spectrum_painter.elf
[ 73%] Built target hackrf_usb_objects
[ 77%] Built target hackrf_usb_ram_objects
[ 80%] Built target hackrf_usb_dfu_objects
[ 97%] Built target application.elf
[ 97%] Built target hackrf_usb.elf
[ 97%] Built target hackrf_usb_ram.elf
[ 97%] Built target hackrf_usb_dfu.elf
[ 97%] Built target application
[ 97%] Built target hackrf_usb_ram.bin
[ 97%] Built target hackrf_usb.bin
[ 97%] Built target hackrf_usb_dfu.bin
[100%] Built target baseband

SO i added a new PR just simply tuned the print order.
This we can see the % in last screen.

@zxkmm zxkmm deleted the real_compiler_ver branch April 30, 2024 07:41
htotoo pushed a commit to htotoo/portapack-mayhem that referenced this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants