poudriere log: http://p.defau.lt/?vrOLjlsBSJP1WDf3XhyQYQ
% portlint -AC
looks fine.
Details
Details
- Reviewers
koobs - Commits
- rP393308: [NEW] www/py-frappe-bench: Frappe / ERPNext apps setup tool
Diff Detail
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| www/py-bench/Makefile | ||
|---|---|---|
| 4 ↗ | (On Diff #7054) | This doesn't appear to be the same as PyPI's bench package, and thus is incorrectly named and conflicts: https://pypi.python.org/pypi/bench
|
| 5 ↗ | (On Diff #7054) | I would look at whether DISTVERION might be better than PORTVERSION here due to the letter in the version |
| 10 ↗ | (On Diff #7054) | Upstream (and setup.py) description is: Benchmark resources usage |
| 30 ↗ | (On Diff #7054) | Some Python ports install setup.py:console_script's (in LOCALBASE/bin), as this one does: https://github.com/frappe/bench/blob/master/setup.py#L21 Try using USE_PYTHON=concurrent, which will ensure that installed script filenames have the correct Python version suffix for the users installed/desired version of Python. Note: Some python packages (not this one) install already version-suffixed scripts. In this case, one should stop (rm) those from being installed, and use USE_PYTHON=concurrent to do it instead. Everything else aside, the end result of a Python port installation (that supports multiple versions of Python) should always be:
|
| www/py-bench/files/patch-setup.py | ||
| 8 ↗ | (On Diff #7054) | Why as this needed? If it's a bug fix, has it been submitted upstream? If so, add patch header breadcrumbs/comments with description and link references |
| 17 ↗ | (On Diff #7054) | I would probably use >= in this case, rather than no version at all. Update the RUN_DEPENDS version accordingly. |
| www/py-bench/Makefile | ||
|---|---|---|
| 4 ↗ | (On Diff #7054) | Sure, submitted. https://github.com/frappe/bench/issues/127 |
| 5 ↗ | (On Diff #7054) | The last release version v0.92 on GitHub needs a lot changes to work, what is the best solution for this kind of ports if I have to use the master branch? can I just the version number in setup.py + date, like PORTVERSION= 0.1.20150713? or is it possible to use the ${GH_TAGNAME} as the port version? |
| 10 ↗ | (On Diff #7054) | This is the package on PyPI :) |
| 30 ↗ | (On Diff #7054) | Thanks for the tip. I will add it. |
| www/py-bench/files/patch-setup.py | ||
| 8 ↗ | (On Diff #7054) | I'm new to this, please correct me if I'm wrong. py_modules=['foo'] installs foo.py to lib/site-packages/foo.py and this port needs to install the bench/ directory. |
| 17 ↗ | (On Diff #7054) | Okay, I will upload a new patch when I got their reply for new port name. Thank your help. |
| www/py-bench/Makefile | ||
|---|---|---|
| 5 ↗ | (On Diff #7054) | The only reason for suggesting DISTVERSION was because of 's' character in PORTVERSION, but I can't tell where it came from. Typo? Intentional? Regardless of the reason, using DISTVERSION for these kinds of non-numeric version strings, automatically derives a 'sane' PORTVERSION value, that is less prone to issues. Specifically, this is related to the requirement for PORTVERSION strings only to 'increase in their values, never decrease (thus requiring a PORTEPOCH bump), and the need to be able to say a version is less-than or greater-than another value, which is more difficult with alphabetic characters in version strings. See: http://www2.au.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/book.html#idp57382352 Key part: Some vendor's distribution names which do not fit into the ${PORTNAME}-${PORTVERSION}-scheme can be handled automatically by setting DISTVERSION. PORTVERSION will be derived from it automatically.Having said all that (above), and since looking at the upstream code, it appears that the version scheme upstream is using is currently X.YZ, and that the version value (0.1) in setup.py is just plain wrong, and needs to be fixed to match the current 'proper' version. An issue should be created for that. This brings us to the last bit, that is, "the latest 'release' version is broken or doesn't have fixes that haven't been released yet'. In this case, something like PORTVERSION.<DATESTAMP> is OK, but only if the new version that is greater-than the previous. I'm not particularly a fan of 'making up version strings', and I tend to use the following alternatives:
TLDR: Upstream needs to fix their packaging, versioning and release practices |
| 10 ↗ | (On Diff #7054) | Sorry, I meant to delete this comment. *shakes fist at phabricator*. Ignore. |
| 27 ↗ | (On Diff #7054) | If you end up getting upstream to tag a new release, use that tag here. |
| www/py-bench/files/patch-setup.py | ||
| 8 ↗ | (On Diff #7054) | I just wanted to understand the need (certainly I know packages=['foo'] is the most common way), and whether the port doesn't work or breaks other things without this patch .. If things don't work without this patch, or the package doesn't install the way, or where it is supposed to according to Python packaging rules/conventions, the upstream should be fixed, and links/references/description added to the patch. |
Comment Actions
Hi @koobs, thanks so much for your help. I really appreciate it.
- changed PORTNAME to py-frapp-bench [0]
- changed PORTVERSION to the latest release 0.92 and removed GH_TAGNAME
- patch-a93acec is a patch of changes from v0.92 to master head
- submitted setup.py py_modules => pkackages to upstream [1]
- updated the version number in setup.py to 0.92 [2]
- submitted contab(1) fix for FreeBSD to upstream [3]
[0]: https://github.com/frappe/bench/issues/127
[1]: https://github.com/frappe/bench/pull/129
[2]: https://github.com/frappe/bench/pull/131
[3]: https://github.com/frappe/bench/pull/132
Comment Actions
s|frapp|frappe|
poudriere log: http://p.defau.lt/?qW72rgomGnEbrkrx8nNEIQ
% portlint -AC
looks fine.
Comment Actions
Thanks @loader, this looks much better! You're doing AWESOME :)
If this builds successfully, im happy for you to commit.
Regarding commit log conventions/formats, I suggest:
category/port: Short summary * Itemized changes * Itemized changes
In NEW PORT cases:
[NEW] category/portname: COMMENT `cat pkg-descr`