Skip to content

Conversation

radityankn
Copy link
Contributor

This script is basically the modification from OpenSUSE Tumbleweed environment install script. The only changes was about the python package names with awk and some renaming to match fedora packages. Tested and the resulting environment is able to compile ArduCopter for Pixhawk1 (STM32) and ESP32S3 Devkit without error. The resulting binaries have not been tested yet

@Ryanf55
Copy link
Contributor

Ryanf55 commented Dec 2, 2024

Always nice to see more of these! Do you mind amending your commit title to prefix it with "Tools: "

@radityankn
Copy link
Contributor Author

Always nice to see more of these! Do you mind amending your commit title to prefix it with "Tools: "

No problem, I will add it

@radityankn radityankn changed the title Add scripts for Fedora systems Tools: Add scripts for Fedora systems Dec 2, 2024
@Ryanf55
Copy link
Contributor

Ryanf55 commented Dec 2, 2024

By commit, I mean here:
image

In CLI:
git commit --amend

change your commit title

Then,

git push -f

For reference: https://ardupilot.org/dev/docs/submitting-patches-back-to-master.html#preparing-commits

@radityankn radityankn closed this Dec 3, 2024
@radityankn radityankn deleted the fedora_script branch December 3, 2024 01:06
@radityankn radityankn restored the fedora_script branch December 3, 2024 01:06
@radityankn radityankn reopened this Dec 3, 2024
@radityankn
Copy link
Contributor Author

radityankn commented Dec 3, 2024

sorry, I accidentally deleted my branch while cleaning up my repo

@Ryanf55
Copy link
Contributor

Ryanf55 commented Dec 3, 2024

No worries. Looks like you also added a merge commit. ArduPilot uses a rebase workflow. I'll drop that off your branch and force push. Looks fine otherwise!

@Ryanf55
Copy link
Contributor

Ryanf55 commented Dec 3, 2024

Alrighty, all cleaned up. Approved! I requested final review from a maintainer.

If you have time, mind trying to add a job in CI to enforce this script can continue to work as you intended as a follow up PR?
We have run through many of the setup scripts in this workflow.
It can be amended to support fedora.
https://github.com/ArduPilot/ardupilot/blob/master/.github/workflows/test_environment.yml

@Ryanf55
Copy link
Contributor

Ryanf55 commented Dec 3, 2024

Eh, maybe not, I don't think we can run these setup scripts tests on fedora in CI:
actions/runner-images#10802

Maybe you know a way?

@radityankn
Copy link
Contributor Author

radityankn commented Dec 3, 2024

I'd like to help, but I don't think I have time to do the CI part. I'm still new to github, still have to learn more about the testing and CI part since I never used it before

anyway, thank you for reviewing the PR

@Ryanf55 Ryanf55 requested a review from peterbarker December 3, 2024 14:08
@peterbarker
Copy link
Contributor

I'm a bit concerned by the fact that this is just the fedora install script which has been copied sideways and modified.

By contrast, our install-prereqs-ubuntu.sh script handles Ubuntu, debian and Mint.

Copying-sideways-and-modifying increases maintenance load on the project, so I would strongly prefer a modification (and perhaps rename) of the existing script to be able to handle both.

Something like

REMOVE_PACKAGE="sudo dnf remove"  # OpenSUSE
if $IS_FEDORA; then
    REMOVE_PACKAGE="sudo zypper rm"
fi

would make it all reasonable clean, I think.

@radityankn
Copy link
Contributor Author

@peterbarker sorry for the late reply. So from your suggestion, it's better to merge the OpenSUSE and Fedora scripts into a single env install file to make maintenance easier, correct?

@radityankn
Copy link
Contributor Author

it can be done with some if statements though, to handle differences in package names and others

@peterbarker
Copy link
Contributor

@peterbarker sorry for the late reply. So from your suggestion, it's better to merge the OpenSUSE and Fedora scripts into a single env install file to make maintenance easier, correct?

That's correct.

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

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

Please rejig as a modification to the opensuse script.

I wouldn't mind if you started by git mv to a different filename first which is collective of both Fedora and OpenSUSE. I'll note that our Ubuntu install script also handles base Debian and Mint, 'though, so it's already a matter of knowing which one to run from some other source, really....

@radityankn
Copy link
Contributor Author

Since it has been quite old, I probably should start with a new script modification, though probably not soon as I'm also quite busy at the moment. Probably better to close this PR before I make a new one with a better script. Thank you for your input @peterbarker

@radityankn radityankn closed this May 15, 2025
@peterbarker
Copy link
Contributor

Since it has been quite old, I probably should start with a new script modification, though probably not soon as I'm also quite busy at the moment. Probably better to close this PR before I make a new one with a better script. Thank you for your input @peterbarker

No worries. Sorry if it seems I'm being too finicky, but whatever goes in has to be maintained!

@Linjieqiang
Copy link
Contributor

Any idea? I use the fedora system now. And want to build the ardupilot firmware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants