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

make: pkg.mk: don't call git am if there are no patches #17858

Merged
merged 1 commit into from
Apr 1, 2022

Conversation

kaspar030
Copy link
Contributor

Contribution description

Don't call git am if there are no patches. Git should gracefully handle it, but somehow doesn't always (like, /dev/null broken).
And, no need to call git if a simple test is sufficient.

Testing procedure

Issues/PRs references

@kaspar030 kaspar030 added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 24, 2022
@github-actions github-actions bot added Area: build system Area: Build system Area: pkg Area: External package ports labels Mar 24, 2022
pkg/pkg.mk Outdated Show resolved Hide resolved
@kaspar030 kaspar030 force-pushed the pkg_no_git_am_without_patches branch from 175c5df to 0dfb264 Compare March 25, 2022 10:51
@kfessel
Copy link
Contributor

kfessel commented Mar 25, 2022

The reason for that <dev/null is a non graceful handling of no file given (git am will try to read a patch from stdin)
The solution of this PR is check if git am should even be called (instead of piping /dev/null into stdin)

another possible source of nothing could be echo -n |

$(Q) echo -n | $(GIT_IN_PKG) $(GITFLAGS) am $(GITAMFLAGS) $(PKG_PATCHES)

not calling if not need seems to be the better solution

@kaspar030
Copy link
Contributor Author

another possible source of nothing could be echo -n |

yeah, but that -n isn't posix?

even /dev/null was a fine solution. I had this fix done for the case where /dev/null was broken, before I understood that, to fix "empty patch" errors from git. I'd also be fine with just closing this. let's not spend more time here

@kfessel
Copy link
Contributor

kfessel commented Mar 25, 2022

yeah, but that -n isn't posix?

posix is seems to be aware of -n https://www.unix.com/man-page/posix/1posix/echo/ but recommends using printf
so printf "" | would do

Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

murdock tested this and it is working as intended

@kaspar030 kaspar030 merged commit 5f597b5 into RIOT-OS:master Apr 1, 2022
@kaspar030 kaspar030 deleted the pkg_no_git_am_without_patches branch April 1, 2022 09:40
@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants