Skip to content

Fix debian/rules makefile: use shell commands instead of dollar replacements#621

Merged
qiluo-msft merged 4 commits intosonic-net:masterfrom
qiluo-msft:qiluo/fixmake
Jun 6, 2020
Merged

Fix debian/rules makefile: use shell commands instead of dollar replacements#621
qiluo-msft merged 4 commits intosonic-net:masterfrom
qiluo-msft:qiluo/fixmake

Conversation

@qiluo-msft
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft commented Jun 6, 2020

because:

  1. make evaluates conditionals (e.g., ifeq) at the time it reads a Makefile.
  2. no need to replace with shell command outputs

ref: https://stackoverflow.com/a/11994561/2514803

replacements because:
1. `ifeq` evaluates conditionals when it reads a Makefile.
2. no need to replace with shell command outputs

ref: https://stackoverflow.com/a/11994561/2514803
jleveque
jleveque previously approved these changes Jun 6, 2020
@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Jun 6, 2020

@qiluo-msft: FYI, I made a small change to your description above. Looks good to me. Nice find!

jleveque
jleveque previously approved these changes Jun 6, 2020
jleveque
jleveque previously approved these changes Jun 6, 2020
@qiluo-msft qiluo-msft merged commit 322dd01 into sonic-net:master Jun 6, 2020
@qiluo-msft qiluo-msft deleted the qiluo/fixmake branch June 6, 2020 04:49
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
…cements (sonic-net#621)

* Fix debian/rules makefile: use shell commands instead of dollar
replacements because:
1. `ifeq` evaluates conditionals when it reads a Makefile.
2. no need to replace with shell command outputs
ref: https://stackoverflow.com/a/11994561/2514803
* Fix one more `echo`
* Revert back one shell replacement
* Fix some bash command and Makefile escaping issue
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