public inbox for gentoo-lisp@lists.gentoo.org
 help / color / mirror / Atom feed
From: "Marijn Schouten (hkBst)" <hkBst@gentoo.org>
To: Stelian Ionescu <sionescu@common-lisp.net>
Cc: gentoo-lisp@lists.gentoo.org
Subject: Re: [gentoo-lisp] common-lisp.eclass cleanup
Date: Tue, 16 Oct 2007 13:04:28 +0200	[thread overview]
Message-ID: <47149ABC.1000408@gentoo.org> (raw)
In-Reply-To: <20071015222846.GA124069@universe.org>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Stelian Ionescu wrote:
> On Mon, Oct 15, 2007 at 10:45:32AM +0200, Christian Faulhammer wrote:
> [snip]
>> Just a cosmetic...stay with one system.  The local variables for Emacs
>> are unncessary, or do you have problems with app-emacs/gentoo-syntax?
> 
> ok, here's the last version of the eclass; sorry for the delay

Comments by me within <-- these are my comments -->

# Copyright 1999-2007 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
#
# Author Matthew Kennedy <mkennedy@gentoo.org>
#
# This eclass supports the installation of Common Lisp libraries

<-- Where are the usage comments? Which are the public functions? -->

inherit eutils

CLSOURCEROOT="${ROOT}"/usr/share/common-lisp/source/
CLSYSTEMROOT="${ROOT}"/usr/share/common-lisp/systems/
<-- Before I really meant to ask about these two variables. I don't understand
what the point is as CLSOURCEROOT will contain symlinks to within
CLSYSTEMROOT. Can you explain this please. -->

<-- Where is the comment about how to override these variables? -->
CLPACKAGE="${PN}"
CLSYSTEMS="${PN}"

DEPEND="virtual/commonlisp"

EXPORT_FUNCTIONS src_install

path-absolute-p() {
	if [ $# -ne 1 ]; then
		die "path-absolute-p must receive exactly one argument"
	fi
    local path="${1}"
    [ "${path:0:1}" == / ]
}

<-- I would prefer:
path-absolute-p() {
	[ $# -ne 1 ] &&	die "path-absolute-p must receive exactly one argument"
	[ "${1:0:1}" == / ]
}

or even

path-absolute-p() {
	assert_arg_num 1 # is bash powerful enough to define and use such a function?
	[ "${1:0:1}" == / ]

and its name should be absolute-path-p.
}

or have it return true only if all its arguments start with '/'.
- -->

common-lisp-install-relatively() {
	if [ $# -lt 1 ] || [ $# -gt 2 ] ; then
		die "common-lisp-install-relatively must receive one or two arguments"
	fi
    local thing="${1}" ; local dir="${2}"
    insinto "${CLSOURCEROOT}/${CLPACKAGE}/${dir}"
    doins -r "${thing}"
}

common-lisp-install() {
	if [ $# -eq 0 ]; then
		die "common-lisp-install must receive at least one argument"
	fi
    local _oldclpackage="${CLPACKAGE}"
    [ "${1}" == "-p" ] && { CLPACKAGE="${2}" ; shift ; shift ; }
	for thing in "$@"; do
        if path-absolute-p "${thing}" ; then
            common-lisp-install-relatively "${thing}"
        else
		    common-lisp-install-relatively "${thing}" "$(dirname "${thing}")"
        fi <-- indentation is screwed up here (and a lot of other places)
because of a combination of spaces and tabs. -->
	done
    CLPACKAGE="${_oldclpackage}"
}

common-lisp-install-single-system() {
	if [ $# -ne 1 ]; then
		die "common-lisp-install-single-system must receive exactly one argument"
	fi
    if [ ! -f "${D}/${CLSOURCEROOT}/${CLPACKAGE}/${1}.asd" ]; then
        die "${D}/${CLSOURCEROOT}/${CLPACKAGE}/${1}.asd does not exist"
    fi
	dosym "${CLSOURCEROOT}/${CLPACKAGE}/${1}.asd" \
		"${CLSYSTEMROOT}/$(basename ${1}).asd"
<-- this is the symlinking that I ask about in the beginning -->
}

common-lisp-system-symlink() {
	dodir "${CLSYSTEMROOT}"
	# if no arguments received, default to
	# the contents of ${CLSYSTEMS}
	if [ $# -eq 0 ]; then
		for package in ${CLSYSTEMS} ; do
			common-lisp-install-single-system "${package}"
		done
	else
        local _oldclpackage="${CLPACKAGE}"
        [ "${1}" == "-p" ] && { CLPACKAGE="${2}" ; shift ; shift ; }
<-- what's the point of setting CLPACKAGE here? I'm not sure I like
influencing common-lisp-install-single-system in that way. -->
        [ $# -eq 0 ] && die "common-lisp-system-symlink needs more arguments"
		for package in "$@" ; do
			common-lisp-install-single-system "${package}"
		done
        CLPACKAGE="${_oldclpackage}"
	fi
}

common-lisp-2_src_install() {
	common-lisp-install *.{lisp,asd}
	common-lisp-system-symlink
	dodoc LICENCE* LICENSE* COPYING* COPYRIGHT README HEADER TODO \
		CHANGELOG ChangeLog BUGS CONTRIBUTORS *NEWS 2> /dev/null
<-- licenses should not be installed separately, /usr/portage/licenses/
contains them already -->
}

# Many of our Common Lisp ebuilds are either inspired by, or actually
# use packages and files from the Debian project's archives.

<-- please remove this stuff. It is misleading and non-functional. -->

do-debian-credits() {
	docinto debian
	for i in copyright README.Debian changelog; do
		[ -f "${S}"/debian/${i} ] && dodoc "${S}"/debian/${i}
	done
	docinto .
}

# Local Variables: ***
# mode: shell-script ***
# tab-width: 4 ***
# End: ***
<-- were these not redundant? -->

Most importantly: add comments.

Marijn

- --
Marijn Schouten (hkBst), Gentoo Lisp project
<http://www.gentoo.org/proj/en/lisp/>, #gentoo-lisp on FreeNode
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.7 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHFJq8p/VmCx0OL2wRAvktAJ9wUftrdekr9fmXEXcQo5RwskHQRQCcDnO2
c0XrE9cjXnwaPV3iKccuxYc=
=7ZwI
-----END PGP SIGNATURE-----
-- 
gentoo-lisp@gentoo.org mailing list



  reply	other threads:[~2007-10-16 11:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-12 21:30 [gentoo-lisp] common-lisp.eclass cleanup Stelian Ionescu
2007-10-13 10:05 ` Marijn Schouten (hkBst)
2007-10-13 11:30   ` Stelian Ionescu
2007-10-15 10:04     ` Marijn Schouten (hkBst)
2007-10-15 13:39       ` Stelian Ionescu
2007-10-13 10:07 ` Marijn Schouten (hkBst)
2007-10-13 10:48   ` Stelian Ionescu
2007-10-15  8:45 ` Christian Faulhammer
2007-10-15 13:14   ` Stelian Ionescu
2007-10-15 22:28   ` Stelian Ionescu
2007-10-16 11:04     ` Marijn Schouten (hkBst) [this message]
2007-10-16 11:27       ` Christian Faulhammer
2007-10-18 14:46       ` Stelian Ionescu
2007-10-18 17:15         ` Marijn Schouten (hkBst)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47149ABC.1000408@gentoo.org \
    --to=hkbst@gentoo.org \
    --cc=gentoo-lisp@lists.gentoo.org \
    --cc=sionescu@common-lisp.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox