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

Simplify and fix install rpms #145

Merged
merged 3 commits into from
Dec 6, 2024
Merged

Conversation

mkisielewski-arista
Copy link
Contributor

From the commit messages:

The rpms-* files all started with lines beginning with '#' which would
suggest a unix-script-style comment, but when the '#' was applied as an
inline comment, the install-rpms script treated that as part of the
package name, breaking it. Having those comments also meant that they
have to be stripped when processing.

This patch moves the content of those comments to a standalone README,
simplifying the sytnax of the file so it can be used as-is in the
install-rpms script.

The install-rpms accepted two non-optional "options" that ended up being
used the same way. There was some logic in that bash script that checked
and processed those options, and as it's typical of bash code, it had a
bug, where if the script was supplied with only one of the two
"options", the value of non-existent $extra_rpms_file, because it was
enclosed in double quotes, would be treated as an actual object (an
empty string), thus making the following code:

for file in "$common_rpms_file" "$extra_rpms_file"; do
(...)
mapfile -t tmp_array < <(awk '!/^#/' "$file")

iterate over an empty "$file" blocking awk.

Because the only thing that this script did was to install RPMs
specified by text files, this patch changes it to do exactly that
without any error-prone bash logic.

The rpms-* files all started with lines beginning with '#' which would
suggest a unix-script-style comment, but when the '#' was applied as an
inline comment, the install-rpms script treated that as part of the
package name, breaking it. Having those comments also meant that they
have to be stripped when processing.

This patch moves the content of those comments to a standalone README,
simplifying the sytnax of the file so it can be used as-is in the
install-rpms script.
The install-rpms accepted two non-optional "options" that ended up being
used the same way. There was some logic in that bash script that checked
and processed those options, and as it's typical of bash code, it had a
bug, where if the script was supplied with only one of the two
"options", the value of non-existent $extra_rpms_file, because it was
enclosed in double quotes, would be treated as an actual object (an
empty string), thus making the following code:
```
for file in "$common_rpms_file" "$extra_rpms_file"; do
(...)
mapfile -t tmp_array < <(awk '!/^#/' "$file")
```
iterate over an empty "$file" blocking awk.

Because the only thing that this script did was to install RPMs
specified by text files, this patch changes it to do exactly that
without any error-prone bash logic.
@aajith-arista aajith-arista merged commit 5018f40 into main Dec 6, 2024
2 checks passed
@aajith-arista aajith-arista deleted the simplify-and-fix-install-rpms branch December 6, 2024 08:25
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.

2 participants