Skip to content

Makefile: generate-files: fix check for empty TMP_OUT#47569

Merged
neersighted merged 1 commit intomoby:masterfrom
thaJeztah:make_fix_empty_check
Mar 15, 2024
Merged

Makefile: generate-files: fix check for empty TMP_OUT#47569
neersighted merged 1 commit intomoby:masterfrom
thaJeztah:make_fix_empty_check

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Mar 15, 2024

commit c655b7d added a check to make sure the TMP_OUT variable was not set to an empty value, as such a situation would perform an rm -rf /** during cleanup.

However, it was a bit too eager, because Makefile conditionals (ifeq) are evaluated when parsing the Makefile, which happens before the make target is executed.

As a result $@_TMP_OUT was always empty when the ifeq was evaluated, making it not possible to execute the generate-files target.

This patch changes the check to use a shell command to evaluate if the var is set to an empty value.

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

Copy link
Copy Markdown
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

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

LGTM, but the commit message needs a tweak ("nerate-files").

commit c655b7d added a check to make sure
the TMP_OUT variable was not set to an empty value, as such a situation would
perform an `rm -rf /**` during cleanup.

However, it was a bit too eager, because Makefile conditionals (`ifeq`) are
evaluated when parsing the Makefile, which happens _before_ the make target
is executed.

As a result `$@_TMP_OUT` was always empty when the `ifeq` was evaluated,
making it not possible to execute the `generate-files` target.

This patch changes the check to use a shell command to evaluate if the var
is set to an empty value.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the make_fix_empty_check branch from 6f2868d to 25c9e6e Compare March 15, 2024 16:55
@thaJeztah
Copy link
Copy Markdown
Member Author

Oh! Good catch, not sure what happened there, maybe autocorrect happened.

@thaJeztah thaJeztah requested a review from neersighted March 15, 2024 16:56
@thaJeztah
Copy link
Copy Markdown
Member Author

racy test?

=== Failed
=== FAIL: amd64.integration.plugin.logging TestReadPluginNoRead/explicitly_enabled_caching (0.88s)
    read_test.go:90: assertion failed: strings.TrimSpace(buf.String()) is not "hello world": []
    --- FAIL: TestReadPluginNoRead/explicitly_enabled_caching (0.88s)

=== FAIL: amd64.integration.plugin.logging TestReadPluginNoRead (4.98s)
    read_test.go:93: [dc43ad45e0969] daemon is not started

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.

3 participants