Re: wip/cliqz: Request for review

[Roland Illig <roland.illig%gmx.de@localhost>]

Am 17.01.2019 um 16:06 schrieb Santhosh Raju:
> On Thu, Jan 17, 2019 at 3:34 AM Roland Illig <roland.illig%gmx.de@localhost> wrote:
>>
>> Several of the DISTFILES don't have a version number in their filename,
>> and I expect that over time, these will receive updates from upstream,
>> especially adult-domains.bin. This will make your package unbuildable
>> because some day the checksums will not match anymore.
>>
>> An immediate fix for this is to define DIST_SUBDIR=${PKGVERSION_NOREV}
>> (see bmake help topic=distfiles) so that other packages are not
>> negatively affected when these files are downloaded to the globally
>> shared DISTDIR.
> 
> Agreed on the DISTFILES being kept in a separate DIST_SUBDIR. I set
> DIST_SUBDIR=${PKGBASE} so that there are no other packages that may
> accidentally use that sub-directory. The changes are in the latest wip
> commit.

Sorry for the typo I made above. I meant DIST_SUBDIR=${PKGNAME_NOREV}
since it includes both the PKGBASE and the PKGVERSION. Chances are high
that in a later version of the package you want to update the Firefox
extensions, therefore it is common to use PKGNAME_NOREV.

>> The CHECK_WRKREF_SKIP looks a bit strange to me.
> 
> This was taken directly from the www/firefox Makefile[1] line 63, and
> when commented out it fails with
> 
> ERROR: *** The above files still have references to the build directory.
> ERROR:     This is possibly an error that should be fixed by unwrapping
> ERROR:     the files or adding missing tools to the package makefile!
> *** Error code 1
> 
> The same error happens in www/firefox too.

My idea for this case was to edit buildinfo.html in-place and replace
${WRKDIR} with a placeholder.

>> I don't like bsdtar to be used in the do-install target. Pkglint already
>> warns about using ${CP} and pax -pe in the *-install targets, but it
>> doesn't know about bsdtar yet. The problem is that when the package is
>> installed as root, the owner/group of the files is set to the
>> information from the archive. The owner/group of the files should always
>> be 0/0, or UNPRIVILEGED_USER for unprivileged pkgsrc builds.
>>
> 
> I see the issue with usage of bsdtar, what would be the suggested
> solution for this?

When you run "chown -R ${ROOT_USER}:${ROOT_GROUP}" after extracting the
archive, everything should be fine. Or it could be REAL_ROOT_USER
instead of ROOT_USER, I'm not sure. Therefore I asked about the
difference between these variables in a separate mail.

>> In post-install, to save system resources at runtime, the second command
>> should be ${ECHO} 'exec ${PREFIX}...' to reduce the number of running
>> processes. <del>This also ensures that the exit code is passed correctly.</del>
>> Alternatively, what about a simple symlink?
>>
> 
> This was taken directly from the www/firefox Makefile[1] lines 97-99,
> a symlink also would have worked, but I just followed the convention
> in www/firefox package.
> 
> Would you suggest creating a symlink over the approach in www/firefox?

@ryoon What were your reasons for creating a wrapper script instead of a
symlink?

Best,
Roland