Bug 257041 - [NEW PORT]: devel/py-lief: Parse, modify and abstract ELF, PE and MachO formats.
Summary: [NEW PORT]: devel/py-lief: Parse, modify and abstract ELF, PE and MachO formats.
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs (Nobody)
URL: https://github.com/lief-project/LIEF
Keywords: feature, needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 2021-07-07 15:38 UTC by Neal Nelson
Modified: 2021-08-17 07:16 UTC (History)
4 users (show)

See Also:
ports: maintainer-feedback+


Attachments
Git diff of new port. (2.08 KB, patch)
2021-07-07 15:38 UTC, Neal Nelson
no flags Details | Diff
Git diff of new port. (2.09 KB, patch)
2021-07-12 12:46 UTC, Neal Nelson
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Neal Nelson 2021-07-07 15:38:58 UTC
Created attachment 226285 [details]
Git diff of new port.

Provide a cross platform library which can parse, modify and abstract ELF, PE and MachO formats.

Main features:

    Parsing: LIEF can parse ELF, PE, MachO, OAT, DEX, VDEX, ART and provides an user-friendly API to access to format internals.
    Modify: LIEF enables to modify some parts of these formats
    Abstract: Three formats have common features like sections, symbols, entry point... LIEF factors them.
    API: LIEF can be used in C, C++ and Python

This will be required by an upcoming update to the currently broken cad/py-cadquery.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-09 08:10:42 UTC
Review items:

- Add python to CATEGORIES
- Use MASTER=SITES=CHEESESHOP (PyPI) unless there is a compelling case not to
- Match COMMENT to upstream setup.py:description (Library to instrument executable formats), or suggest upstream improve their description.
- Use USE_PYTHON=autoplist unless there is a compelling case not to. pkg-plist not requires after this change
- Use USE_PYTHON=concurrent for Python packages which install files into common (non python version specific) locations. THis port does (LOCALBASE/bin/*)
- Remove do-build: target, shouldn't be necessary with USE_PYTHON=distutils
- Strip shared library in a post-install: target

See also: https://github.com/lief-project/LIEF/issues/575
Comment 2 Neal Nelson 2021-07-09 16:52:57 UTC
(In reply to Kubilay Kocak from comment #1)

- I assume that python is a virtual category? This does not bode well for my other port submissions, which are also python and also not tagged as such.
- I chose github as this is where the project is based. Is it a policy to use pypi if available even though there may be more up to date version available elsewhere? If so, see above comment about not boding well.
- That's a better comment (yours, not mine). It's not shown on github.
- Alas autoplist does not work, for the very reason of the github issue you linked: the autoplist is incorrectly generated. It works fine as submitted.
- I'm not exactly sure what the ramifications of USE_PYTHON=concurrent are, but without autoplist it may do nothing.
- Sorry for the do-build. This was copied over from a port that needed it.
- Strip added.
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-11 01:32:26 UTC
(In reply to Neal Nelson from comment #2)

 - Yep, its a virtual category. Please add them

 - Yep, it's policy, but the 'reason' is 'sdists via pypi' tests the end-to-end Python packaging workflows and best-practices of the upstream project. In this case, its highlighted their sdists (and pypi entry) are not up to date. Using sources other than PyPI if there is a "compelling temporary case" [1] is fine. Examples include, desired files (tests, licences) are not packaged in the sdist, or the latest version is not (yet) uploaded to PyPI. Please ask upstream to fix these issues

 - Mentioned comment is upstreams not mine  :) (setup.py:description usually). 

 - autoplist using a pkg-plist is fine until that's resolved. If its possible to fix an issue in a local port patch until the next upstream release is cut, that's preferred. 

 - concurrent handles most common-location filename renaming in both autoplist and static plist cases.

You'll want to enable DEVELOPER=yes in make.conf and run tests with portlint -AC and poudriere for correct and complete QA. We're available on #freebsd-ports on Libera IRC if you need help
Comment 4 Dan Langille freebsd_committer 2021-07-11 01:33:38 UTC
If you hit any issues and feel like it's hard, ask. It's a complex environment.
Comment 5 Neal Nelson 2021-07-12 12:46:14 UTC
Created attachment 226389 [details]
Git diff of new port.

Here's a new version of the port with all the suggested changes.

It's been portlinted and poudriere testported, as before.

I have a couple of other new port submission that I will update with the same changes where appropriate. These are all dependent ports on a large change to cad/OCP and cad/cadquery that I am waiting to submit. cad/OCP currently is broken because cad/opencascade changed. The dependent ports are build dependencies and to be frank I have no other interest in them, including this port, other than that they have to exist for the build to work.

The other prs are: bug #256924  and bug #256925.
Comment 6 Fukang Chen freebsd_committer 2021-07-13 02:25:36 UTC
It's unable to generate a correct ${_PYTHONPKGLIST} with USE_PYTHON=autoplist 

# make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: %%PYTHON_SITELIBDIR%%/lief%%PYTHON_EXT_SUFFIX%%.so
===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: %%PYTHON_SITELIBDIR%%/.11.5/lief%%PYTHON_EXT_SUFFIX%%.so
===> Error: Plist issues found.
*** Error code 1

Stop.
make: stopped in /usr/ports/devel/py-lief

There's a weird path in ${_PYTHONPKGLIST}:
/usr/local/lib/python3.8/site-packages/.11.5/lief.cpython-38.so

The weird path was caused by _mutate_outputs() in distutils/command/install_lib.py:
https://github.com/python/cpython/blob/v3.8.10/Lib/distutils/command/install_lib.py#L143-L156
   143      def _mutate_outputs(self, has_any, build_cmd, cmd_option, output_dir):
   144          if not has_any:
   145              return []
   146
   147          build_cmd = self.get_finalized_command(build_cmd)
   148          build_files = build_cmd.get_outputs()
   149          build_dir = getattr(build_cmd, cmd_option)
   150
   151          prefix_len = len(build_dir) + len(os.sep)
   152          outputs = []
   153          for file in build_files:
   154              outputs.append(os.path.join(output_dir, file[prefix_len:]))
   155
   156          return outputs

on my 14.0-CURRENT aarch64:

build_dir => build/lib.freebsd-14.0-CURRENT-arm64-3.8
file => /usr/ports/devel/py-lief/work-py38/LIEF-0.11.5/lief.cpython-38.so
prefix_len => len(build_dir) + len(os.sep) => 40 + 1 => 41
file[prefix_len:] => .11.5/lief.cpython-38.so
output_dir => /usr/ports/devel/py-lief/work-py38/stage/usr/local/lib/python3.8/site-packages/

os.path.join(output_dir, file[prefix_len:]) =>
/usr/ports/devel/py-lief/work-py38/stage/usr/local/lib/python3.8/site-packages/.11.5/lief.cpython-38.so

It looks like we are supposed to get a file name
build/lib.freebsd-14.0-CURRENT-arm64-3.8/lief.cpython-38.so
instead of 
/usr/ports/devel/py-lief/work-py38/LIEF-0.11.5/lief.cpython-38.so

https://github.com/python/cpython/blob/v3.8.10/Lib/distutils/command/build_ext.py#L637-L664
   637      # -- Name generators -----------------------------------------------
   638      # (extension names, filenames, whatever)
   639      def get_ext_fullpath(self, ext_name):
   640          """Returns the path of the filename for a given extension.
   641
   642          The file is located in `build_lib` or directly in the package
   643          (inplace option).
   644          """
   645          fullname = self.get_ext_fullname(ext_name)
   646          modpath = fullname.split('.')
   647          filename = self.get_ext_filename(modpath[-1])
   648
   649          if not self.inplace:
   650              # no further work needed
   651              # returning :
   652              #   build_dir/package/path/filename
   653              filename = os.path.join(*modpath[:-1]+[filename])
   654              return os.path.join(self.build_lib, filename)
   655
   656          # the inplace option requires to find the package directory
   657          # using the build_py command for that
   658          package = '.'.join(modpath[0:-1])
   659          build_py = self.get_finalized_command('build_py')
   660          package_dir = os.path.abspath(build_py.get_package_dir(package))
   661
   662          # returning
   663          #   package_dir/filename
   664          return os.path.join(package_dir, filename)

inplace == 0
get_ext_fullpath(ext.name) => 
build/lib.freebsd-14.0-CURRENT-arm64-3.8/lief.cpython-38.so

inplace == 1
get_ext_fullpath(ext.name) => 
/usr/ports/devel/py-lief/work-py38/LIEF-0.11.5/lief.cpython-38.so

We could change the inlpace value to 0 in ${WRKSRC}/setup.cfg,
this should make USE_PYTHON=autoplist work:

post-patch:
        @${REINPLACE_CMD} -e 's|^inplace=1|inplace=0|' ${WRKSRC}/setup.cfg
Comment 7 Fukang Chen freebsd_committer 2021-07-13 02:26:35 UTC
https://github.com/lief-project/LIEF/blob/0.11.5/setup.py#L110
100        jobs = self.parallel if self.parallel else 1

We might need to add a --parallel option to respect MAKE_JOBS_NUMBER
PYDISTUTILS_BUILDARGS+=         build_ext --parallel=${MAKE_JOBS_NUMBER}
Comment 8 Fukang Chen freebsd_committer 2021-07-13 05:53:30 UTC
(In reply to Fukang Chen from comment #7)

> PYDISTUTILS_BUILDARGS+=         build_ext --parallel=${MAKE_JOBS_NUMBER}

This should be only --parallel without the build_ext command
PYDISTUTILS_BUILDARGS+=           --parallel=${MAKE_JOBS_NUMBER}
Comment 9 Neal Nelson 2021-07-13 08:11:35 UTC
(In reply to Fukang Chen from comment #6)

This must be an artefact of running on either 14.0-CURRENT or aarch64, or maybe both, as it does not exhibit on 13.0-RELEASE-p3 amd64.
Comment 10 Fukang Chen freebsd_committer 2021-07-13 11:15:20 UTC
(In reply to Neal Nelson from comment #9)

comment #6 is not about reporting the port fails to build on 14.0-CURRENT aarch64,
I was just trying to explaining why USE_PYTHON=autoplist doesn't work.

OS version and architecture are irrelevant to USE_PYTHON=autoplist,
I will just use the testport log from dvl@ here
https://gist.github.com/dlangille/78902092fd8839bc4514492f949bad16

on my 14.0-CURRENT aarch64:
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: %%PYTHON_SITELIBDIR%%/lief%%PYTHON_EXT_SUFFIX%%.so
===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: %%PYTHON_SITELIBDIR%%/.11.5/lief%%PYTHON_EXT_SUFFIX%%.so
===> Error: Plist issues found.

dvl@'s testport log
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: %%PYTHON_SITELIBDIR%%/lief%%PYTHON_EXT_SUFFIX%%.so
===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: %%PYTHON_SITELIBDIR%%/f-0.11.4/lief%%PYTHON_EXT_SUFFIX%%.so
===> Error: Plist issues found.

https://github.com/python/cpython/blob/v3.8.10/Lib/distutils/command/install_lib.py#L143-L156
in the function _mutate_outputs():

build_dir => build/lib.freebsd-14.0-CURRENT-arm64-3.8
build_dir => build/lib.freebsd-12.2-RELEASE-p2-amd64-3.8

file => /usr/ports/devel/py-lief/work-py38/LIEF-0.11.5/lief.cpython-38.so
file => /wrkdirs/usr/ports/vrt/py-lief/work-py38/lief-0.11.4/lief.cpython-38.so

prefix_len => len(build_dir) + len(os.sep) => 40 + 1 => 41
prefix_len => len(build_dir) + len(os.sep) => 43 + 1 => 44

file[prefix_len:] =>    .11.5/lief.cpython-38.so
file[prefix_len:] => f-0.11.4/lief.cpython-38.so

os.path.join(output_dir, file[prefix_len:]) =>
/usr/ports/devel/py-lief/work-py38/stage/usr/local/lib/python3.8/site-packages/.11.5/lief.cpython-38.so
os.path.join(output_dir, file[prefix_len:]) =>
/wrkdirs/usr/ports/vrt/py-lief/work-py38/stage/usr/local/lib/python3.8/site-packages/f-0.11.4/lief.cpython-38.so
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2021-07-13 11:52:05 UTC
(In reply to Fukang Chen from comment #6)

Nice isolation Fukang. And yeh, this is a common class of issue for upstreams that modify or subclass the default installation routines, particular for c extensions.

Feel free to address this in whatever way you think is most suitable, with preference for fixing upstream source to correct the error (rather than special or custom build/install targets (ideally).

We recently fixed one [1] (IRC) by removing 'inplace' overrides from the setup.py which was sufficient to correct the issue. They were also 'double building', which was more problematic.

Regarding the parallel builds, go for it, though it would be nice to have a common (python) way to do this, and I've reached out to upstream to find out if there's options in that regard.


[1] https://github.com/overviewer/Minecraft-Overviewer