Page MenuHomeFreeBSD

new port: net-mgmt/p5-Thruk: Monitoring webinterface for Naemon, Nagios, Icinga and Shinken (need advice)
Needs ReviewPublic

Authored by dvl on Nov 14 2019, 4:25 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Trying to create a port for this and having trouble with sed in theirconfigure.They use 'sed -i' at https://github.com/sni/Thruk/blob/master/script/thruk_set_standard_header#L20sed -i $file.head -e 's/\ \&\&.*examples.*\]\]//g'sed -i $file.head -e 's%:[^:]*conf/lib%%g'

The errors are:

sed: rename(): No such file or directory
created cached js/css files
Can't exec "./script/thruk_set_standard_header": No such file or directory at ./Makefile.PL line 161.
failed to update standard headers at ./Makefile.PL line 163.

  • Error code 2

Running that script manually from within testport -i gives:

root@120R-dvl:/wrkdirs/usr/ports/net-mgmt/p5-Thruk/work/Thruk-d0fc90f # ./script/thruk_set_standard_header
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin
sed: -I or -i may not be used with stdin

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27515
Build 25745: arc lint + arc unit

Event Timeline

dvl created this revision.Nov 14 2019, 4:25 PM
linimon retitled this revision from Trying to create a port for this and having trouble with sed in their configure. They use 'sed -i' at https://github.com/sni/Thruk/blob/master/script/thruk_set_standard_header#L20 sed -i $file.head -e 's/\ \&\&.*examples.*\]\]//g' sed -i $file. to new port: net-mgmt/p5-Thruk: (need advice).Nov 15 2019, 2:31 AM
linimon edited the summary of this revision. (Show Details)
linimon retitled this revision from new port: net-mgmt/p5-Thruk: (need advice) to new port: net-mgmt/p5-Thruk: Monitoring webinterface for Naemon, Nagios, Icinga and Shinken (need advice).
linimon added a subscriber: linimon.

Fixed title. fwiw it should be 'thruk' all lowercase to fit in with existing practice.

linimon removed a subscriber: linimon.Nov 16 2019, 1:42 AM
dvl added a comment.Jan 28 2020, 8:29 PM

I see this works:

$ echo 'aaa bbb ccc' > ~/tmp/sed-test.txt
$ cat ~/tmp/sed-test.txt
aaa bbb ccc
$ sed -e 's/bbb/BBB/g'  ~/tmp/sed-test.txt
aaa BBB ccc
$ sed -i backkk -e 's/bbb/BBB/g'  ~/tmp/sed-test.txt
$ cat ~/tmp/sed-test.txt
aaa BBB ccc
$ 

Perhaps instead of

sed -i $file.head -e 's/\ \&\&.*examples.*\]\]//g'
sed -i bak -e 's/\ \&\&.*examples.*\]\]//g'
dvl added a comment.Jan 28 2020, 8:30 PM

There seems to be a lot of them:

[dvl@ava-pkg-01prd:/usr/local-data/dvl/ports-head/net-mgmt/p5-Thruk/work] $ grep -r 'sed -i' *
Thruk-d0fc90f/docs/documentation/omd-multisite.asciidoc:sed -i.bak 's/enforcing/disabled/' /etc/selinux.config
Thruk-d0fc90f/plugins/plugins-available/panorama/scripts/disable_debug.sh:    sed -i '/TP.tracelog.*console.*temporary debug/d' $file
Thruk-d0fc90f/plugins/plugins-enabled/panorama/scripts/disable_debug.sh:    sed -i '/TP.tracelog.*console.*temporary debug/d' $file
Thruk-d0fc90f/debian/rules:	sed -i -e 's/^.*#su/    su/' debian/tmp/etc/logrotate.d/thruk-base; \
Thruk-d0fc90f/debian/thruk-base.postrm:      sed -i -e '/Include \/usr\/share\/thruk\/thruk_cookie_auth.include/d' $file
Thruk-d0fc90f/debian/thruk-base.prerm:    sed -i -e '/thruk_cookie_auth.include/d' /etc/apache2/sites-available/default*
Thruk-d0fc90f/debian/thruk-base.postinst:          sed -i -e 's|</VirtualHost>|\n    Include /usr/share/thruk/thruk_cookie_auth.include\n</VirtualHost>|g' $file
Thruk-d0fc90f/.travis.yml:  - sed -i '/Test::JavaScript/d' ./Makefile.PL
Thruk-d0fc90f/support/thruk.spec:    sed -i -e 's/^.*#su/    su/' %{buildroot}/%{_sysconfdir}/logrotate.d/thruk-base
Thruk-d0fc90f/t/scenarios/logfile_cache_icinga2_ido_e2e/omd/playbook.yml:  - shell: sed -i -e 's/^skip-networking/#skip-networking\nbind-address = 0.0.0.0\n/g' /opt/omd/sites/demo/.my.cnf
Thruk-d0fc90f/t/scenarios/sakuli_e2e/dropbox_uploader.sh:        sed -i'' 's/:/=/' "$CONFIG_FILE" && source "$CONFIG_FILE" 2>/dev/null
Thruk-d0fc90f/script/disable_debug_timer.sh:  sed -i \
Thruk-d0fc90f/script/thruk_version.sh:sed -i support/thruk.spec -e 's/^Release:.*$/Release: '$rpmrelease'/'
Thruk-d0fc90f/script/thruk_set_standard_header:    sed -i $file.head -e 's/\ \&\&.*examples.*\]\]//g'
Thruk-d0fc90f/script/thruk_set_standard_header:    sed -i $file.head -e 's%:[^:]*conf/lib%%g'
Thruk-d0fc90f/script/enable_debug_timer.sh:  sed -i \
mat added a comment.Jan 28 2020, 8:35 PM

The *_DEPENDS are out of order, they have to happen earlier.

Also, our style make it having those variable defined at once, in a long continuation, not using +=.

net-mgmt/p5-Thruk/Makefile
6

Default, remove.

dvl added a comment.Jan 28 2020, 8:39 PM
In D22372#513374, @mat wrote:

The *_DEPENDS are out of order, they have to happen earlier.
Also, our style make it having those variable defined at once, in a long continuation, not using +=.

If I can ever solve the sed issues, I will fix the above.

woodsb02 added a subscriber: woodsb02.EditedJan 28 2020, 10:20 PM

Typically there are 2 main options to fix the “sed -i” issue:

  1. Add post-patch: target to port Makefile to replace them all

Example port: math/cvc4/Makefile

post-patch:
      @${REINPLACE_CMD} -e "s|sed -i 's|sed -i '' 's|g" \
      ${WRKSRC}/src/fix-install-headers.sh
  1. Make gsed a dependency and add post-patch target to convert sed to gsed

Example port: devel/icmake/Makefile

BUILD_DEPENDS= gsed:textproc/gsed
post-patch:
      @${REINPLACE_CMD} -e 's|sed|gsed|g' \
              ${WRKSRC}/scripts/conversions \
              ${WRKSRC}/scripts/convert

For option 2, be sure that the gsed commands are being used during build phase (not runtime), or ensure gsed is a runtime dependency if required.

Note that not all of those sed commands need to be fixed (e.g. the ones in the debian/ folder can be ignored, not sure about those in the t/scenarios folder?).

Also note that there are a few places in the Thruk code where the -i argument is not the first argument in the sed command, and also a few places where there are multiple -i arguments. You would therefore need to be careful with the syntax provided in option 1 above (it wouldn’t work as it is).
https://github.com/sni/Thruk/blob/9dbb90daa3373c666332f97c94e2695fdd9540ec/script/thruk_version.sh#L55

mat added a comment.Jan 29 2020, 7:52 AM

Example port: math/cvc4/Makefile

post-patch:
      @${REINPLACE_CMD} -e "s|sed -i 's|sed -i '' 's|g" \
      ${WRKSRC}/src/fix-install-headers.sh

This goes against the porter's handbook, 4.4.3. Simple Automatic Replacements clearly states that any replacement that is static must be done with a patch file.