Skip to content

Comments

Fixed incompatibility with BSD Unix sed in makefile (closes #749)#751

Merged
rfelber merged 4 commits intomainfrom
bugfix/issue_749
Oct 22, 2021
Merged

Fixed incompatibility with BSD Unix sed in makefile (closes #749)#751
rfelber merged 4 commits intomainfrom
bugfix/issue_749

Conversation

@Weltraumschaf
Copy link
Member

@Weltraumschaf Weltraumschaf commented Oct 21, 2021

Description

This fixes #749 incompatibility with BSD Unix sed.

Checklist

  • Test your changes as thoroughly as possible before you commit them. Preferably, automate your test by unit/integration tests.
  • Make sure npm test runs for the whole project.
  • Make codeclimate checks happy

Sedon BSD Unix requires an argument for the -i switch. Unless you
wanta backup file inemust add an empty string argument.

Signed-off-by: Sven Strittmatter <[email protected]>
It is not necessar to add ;\ at the end of each line because make
runs each command step by step. So this is removed.

Also echo what is done is not necessary because make prints by
default each command for debug purpose.

Also removed the cd <subdir> because this is brittle: If the cd
commandfails the subsequent commands work in a wrong directory.
Better solution is to always use theconcrete path relative to
the Makefile.

Signed-off-by: Sven Strittmatter <[email protected]>
rsync is for copy files between remote machines. Since we copy
files local it makes no sense to use rsync here. A simple recursive
copy is well suited for this.

Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf Weltraumschaf self-assigned this Oct 21, 2021
@Weltraumschaf Weltraumschaf added the bug Bugs label Oct 21, 2021
@Weltraumschaf Weltraumschaf marked this pull request as draft October 21, 2021 09:28
The -i Switch (in place file processing) is not POSIX compliant
and behaves differently on some systems:

* BSD Unix: requires an argument (e.g. -i '.bak')
* GNU Linux: does not allow an argument

This patch introduces a platform independent way for replacing
the needle stringin with the scanner name in the copied template
files.

The trick is to use a temporary file to save the sed result. First
aproach was to use > to redirect the sed output from STDOUT into
the temp file. This does not work because find's -exec does not
execute a shell and executes the command directly in a forked
child process. So there are no shell features like redirect or
pipe. The solution is to use the sed command 'w' to write the file
and supress the output on STDOUT.

Signed-off-by: Sven Strittmatter <[email protected]>
@Weltraumschaf Weltraumschaf marked this pull request as ready for review October 21, 2021 14:39
Copy link
Member

@malexmave malexmave left a comment

Choose a reason for hiding this comment

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

Works under OS X and seems to do what it's supposed to do. 👍

@rfelber rfelber changed the title Bugfix/issue 749 Fixed incompatibility with BSD Unix sed in makefile (closes #749) Oct 22, 2021
@rfelber rfelber added the ci Changes to the continuous integration setup label Oct 22, 2021
@rfelber rfelber added this to the v3.3.0 milestone Oct 22, 2021
@rfelber rfelber merged commit 2092f48 into main Oct 22, 2021
@rfelber rfelber deleted the bugfix/issue_749 branch October 22, 2021 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bugs ci Changes to the continuous integration setup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Makefile-based scanner creation throws error

3 participants