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

Makefile: Do not rebuild the sidecar if not the default path #59

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

martinetd
Copy link
Contributor

(This is a follow-up on #56 -- it contains #56 itself as that'd conflict so leaving it as draft fo now)

If the SIDECAR variable has been overriden then running cargo is not
useful nor desired:
define a DEFAULT_SIDECAR variable we can compare against and only add
the $(SIDECAR):: rebuild rule if SIDECAR has the default value.


The last commit is complete garbage, but I couldn't come up with a way to flag that I don't want to rebuild the sidecar (the :: make target will always try to run cargo and that'll fail for me); I think I'll just patch it on the nixos side but bringing it up here if you have a better idea.

yeah, this is not good for "normal" build path. What we can do, perhaps, is to have DEFAULT_SIDECAR vs SIDECAR that can be overridden, and if they don't match, don't even trigger cargo (as it's pointless anyways). Let's do this as a separate PR still?

That makes a lot of sense, with the path changed it's not like cargo would be able to do anything useful anyway.

I've fiddled with it a bit and it appears to work with both default and non-default value in my environment.

src/Makefile Outdated Show resolved Hide resolved
If the SIDECAR variable has been overriden then running cargo is not
useful nor desired:

define a DEFAULT_SIDECAR variable with the path that we can build with
cargo: if the user overrides the default value then that rule is not
used and will not invoke cargo.

Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
@martinetd martinetd marked this pull request as ready for review February 8, 2024 23:59
@anakryiko anakryiko merged commit 92952e0 into anakryiko:master Feb 9, 2024
2 checks passed
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