Summary: | [maintainer] emulators/dynagen: clean sanity check | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | Ports & Packages | Reporter: | Pavel Volkov <pavelivolkov> | ||||||||||||
Component: | Individual Port(s) | Assignee: | Kurt Jaeger <pi> | ||||||||||||
Status: | Closed FIXED | ||||||||||||||
Severity: | Affects Some People | CC: | pavelivolkov, pi | ||||||||||||
Priority: | --- | Keywords: | patch, patch-ready | ||||||||||||
Version: | Latest | ||||||||||||||
Hardware: | Any | ||||||||||||||
OS: | Any | ||||||||||||||
Attachments: |
|
This patch needs to be regenerated based on recent commits to the port. 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
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. moving back to triage -- it was promoted accidentally while waiting for feedback. 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. 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. :) 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. 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. 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. 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.
I'll promote this. It has not been tested but visibly it looks good. build-test worked on 10.1a, 9.3a, 8.4i. 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 Committed, thanks! |
Created attachment 148057 [details] [path] emulators/dynagen - Clean 'sanity' check - Use REINPLACE_CMD instead separate patches