Comment 1 John Marino 2014-10-31 18:25:15 UTC

This patch needs to be regenerated based on recent commits to the port.

Comment 2 John Marino 2014-11-14 10:36:31 UTC

update title to indicate this is a maintainer patch.

Pavel -- the PR is stuck because we are waiting for you to update it.  The patch will not apply as it is.

Created attachment 149472 [details]
this new, updated path

Comment 4 John Marino 2014-11-16 11:23:53 UTC

Should I assume that USES+=shebangfix doesn't work for some reason?  You should tell us why you are using REINPLACE_CMD instead of shebangfix so we know it's intentional and necessary.

Comment 5 John Marino 2014-11-19 09:41:09 UTC

moving back to triage -- it was promoted accidentally while waiting for feedback.

Comment 6 John Marino 2015-02-06 16:11:07 UTC

This is the last time I'm going to "ping" this PR, which has been waiting for feedback since 16 Nov 2014.  If not response comes I'll just close it.

Created attachment 152705 [details]
This is a new patch, with 'shebangfix'.

Hello all. Hi John. Sorry for my late reply.
The 'shebangfix' is a good.
But, it takes into account only one search pattern instead of four.
See new patch. Old patch is a better. :)

You may choose one of them independently.

Comment 10 John Marino 2015-02-09 12:08:59 UTC

I like the new patch better, but I don't understand why this line didn't change:

"${FIND} ${WRKSRC} -type f -name \*.py -or -name dynagen -or -name pemu-start.sh ..."


The shebang files are "SHEBANG_FILES=	*.py dynagen pemu-start.sh"

which is exactly the same.  surely these files don't need to be fixed twice?  (e.g. at least dynagen and pemu-start.sh would be on one method or the other, but not both, right?)

Look.

'*.py' it is confConsole.py, console.py, dynamips_lib.py, pemu_lib.py
matched 'shebangfix'.
And pemubin.py, pemuwrapper.py matched 'REINPLACE_CMD'.
And configobj.py, validate.py don't matched all.
'*.py' it's simple and correctly works in all case.

The file 'dynagen', matched 'shebangfix' in first line,
and matched 'REPLACE CMD' in 44 line.
It should be present in all case.

File 'pemu-start.sh' does not match 'shebangfix' at the moment.
But what will happen then?

I wanted the lists of files were the same in all cases.

Old patch easier and better. :)

Comment 12 John Marino 2015-02-09 17:37:16 UTC

so 
1) remove "pemu-start.sh" from shebang line
2) replace "*.py" with "pemubin.py, pemuwrapper.py" on replace line

No it's not easier but it's (more) clear to the next person which files are getting changed by what.  

Honestly, it seems that this REPLACE_CMD is replacing unrelated things mainly to limit itself to one command.  "-e "s|^CONFIGSPECPATH = .*|CONFIGSPECPATH = \['${PREFIX}/share/dynagen'\]|"" has nothing to do with shebang, so I'd really prefer a second, dedicated replace_cmd for dynagen

again -- I know it's not easier to change, but it's more maintainable IMO.

Created attachment 152995 [details]
This is a new patch, with use only 'shebangfix'.

Hi. Sorry. Too many work.
Look last patch.
The 'shebangfix' may be preferred. You was right. :)
This is fun.

Comment 15 John Marino 2015-02-15 21:10:07 UTC

so close!

the patch to this: setup.py

It's hardcoded to python2.7 when python3 is allowed.

I recommend you remove the patch and use REIMPLACE_CMD with a python variable and just change it with sed.

At a quick glance, everything else looked better (ok) to me.

Comment 16 John Marino 2015-02-23 10:49:59 UTC

Pavel, are you going to make an update?  I think it would get the PR promoted, otherwise I'll remove myself from CC list since it seems to be stalled.

Hi. I apologize. I am sick. I'll take a time out. And you can remove your e-mail from CC. :) Sorry. Bye.

Comment 18 John Marino 2015-02-23 16:41:18 UTC

hmm, too sick to fix a single line?

That's a shame, get well soon!

(I've got a million things to do myself otherwise I might try to fix it myself, but I can't right now)

Created attachment 154615 [details]
This is a new patch, with use only 'shebangfix', and single line.

Comment 20 John Marino 2015-03-22 17:15:34 UTC

I'll promote this.  It has not been tested but visibly it looks good.

Comment 21 Kurt Jaeger 2015-03-28 14:37:19 UTC

build-test worked on 10.1a, 9.3a, 8.4i.

Comment 22 commit-hook 2015-03-28 14:39:27 UTC

A commit references this bug:

Author: pi
Date: Sat Mar 28 14:38:51 UTC 2015
New revision: 382486
URL: https://svnweb.freebsd.org/changeset/ports/382486

Log:
  emulators/dynagen: shebangfix for python in the scripts instead of patches

  PR:		194215
  Submitted by:	Pavel Volkov <pavelivolkov@gmail.com> (maintainer)
  Reviewed by:	marino

Changes:
  head/emulators/dynagen/Makefile
  head/emulators/dynagen/files/patch-confConsole.py
  head/emulators/dynagen/files/patch-console.py
  head/emulators/dynagen/files/patch-dynagen
  head/emulators/dynagen/files/patch-dynamips_lib.py
  head/emulators/dynagen/files/patch-pemu_lib.py
  head/emulators/dynagen/files/patch-setup.py
  head/emulators/dynagen/files/setup.py

Comment 23 Kurt Jaeger 2015-03-28 14:40:12 UTC

Committed, thanks!