Re: [gentoo-dev] git-2.eclass final review
* Mike Frysinger schrieb am 23.03.11 um 00:08 Uhr:
2011/3/22 Tomáš Chvátal:
Dne 22.3.2011 22:26, Mike Frysinger napsal(a):
# @BLURB: This eclass provides functions for fetch and unpack git
repositories
fetching/unpacking
Yarp fixed.
well, the fix broke the blurb. it has to be on one line.
# @BLURB: foo
EGIT_BRANCH=${x:-${EGIT_BRANCH:=${EGIT_MASTER}}}
EGIT_COMMIT=${x:-${EGIT_COMMIT:=${EGIT_BRANCH}}}
doesnt make much sense to use := ... it should be :-
[[ $# -ne 1 ]] die ${FUNCNAME}: requires 1 argument (path)
quoting doesnt make much sense ... -ne compares an int, not a string
If using [[ you never need to quote anyway.
-Marc
--
8AAC 5F46 83B4 DB70 8317 3723 296C 6CCA 35A6 4134
Re: [gentoo-dev] git-2.eclass final review
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Dne 31.3.2011 06:55, Jeroen Roovers napsal(a): On Wed, 23 Mar 2011 01:42:51 +0100 Tomáš Chvátal scarab...@gentoo.org wrote: Again the diff is: http://tinyurl.com/62eb88b Why not attach it? What the hell does that URL lead me to? jer Because our git webservice allows us to do nice colorfull diffing, but sadly that url is by default really long...? You know since we gentoo developers usually post tinyurled porn sites links to -dev you just can't trust anything I post here... -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAk2UH/sACgkQHB6c3gNBRYc7/ACfTfZBFaaWZ6ttc2Yv6J9Azet7 W/cAn2K5lB390SPGh2pSDTyiCDkalxia =N3T6 -END PGP SIGNATURE-
Re: [gentoo-dev] git-2.eclass final review
On 31-03-2011 08:32:27 +0200, Tomáš Chvátal wrote: Dne 31.3.2011 06:55, Jeroen Roovers napsal(a): On Wed, 23 Mar 2011 01:42:51 +0100 Tomáš Chvátal scarab...@gentoo.org wrote: Again the diff is: http://tinyurl.com/62eb88b Why not attach it? What the hell does that URL lead me to? Because our git webservice allows us to do nice colorfull diffing, but sadly that url is by default really long...? Point remains that some of us have smart enough mail clients to do the colourfull diffing based on an attached diff file inline. As you can understand, that is much more convenient (saves a click to another window) and also persists in time. Extra bonus, allows to easily reply to it using inline comments to the diff as well, without having to load it up in the editor first. IOW: if you want to post an url, fine, but just include the diff in your email please. Thanks. -- Fabian Groffen Gentoo on a different level
Re: [gentoo-dev] git-2.eclass final review
2011/3/31 Tomáš Chvátal scarab...@gentoo.org: Dne 31.3.2011 06:55, Jeroen Roovers napsal(a): On Wed, 23 Mar 2011 01:42:51 +0100 Tomáš Chvátal scarab...@gentoo.org wrote: Again the diff is: http://tinyurl.com/62eb88b Why not attach it? What the hell does that URL lead me to? Because our git webservice allows us to do nice colorfull diffing, but sadly that url is by default really long...? Will the tinyurl link last forever? Matt
Re: [gentoo-dev] git-2.eclass final review
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 On 03/31/2011 12:00 PM, Matt Turner wrote: 2011/3/31 Tomáš Chvátal scarab...@gentoo.org: Dne 31.3.2011 06:55, Jeroen Roovers napsal(a): On Wed, 23 Mar 2011 01:42:51 +0100 Tomáš Chvátal scarab...@gentoo.org wrote: Again the diff is: http://tinyurl.com/62eb88b Why not attach it? What the hell does that URL lead me to? Because our git webservice allows us to do nice colorfull diffing, but sadly that url is by default really long...? Will the tinyurl link last forever? Matt Will the proper git URL last forever? (^_^) - - Aaron -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.17 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREIAAYFAk2U8MAACgkQCOhwUhu5AEn8xwEAvMnl3cbGVKWiUjLDe7UQelTz zLcdbwz+BKbkfltN6VkBAJHhZCOfwZZglPqt89oBZrY7oYT3v7LyKzr2O5D/MR0u =shYz -END PGP SIGNATURE-
Re: [gentoo-dev] git-2.eclass final review
On Wed, 23 Mar 2011 01:42:51 +0100 Tomáš Chvátal scarab...@gentoo.org wrote: Again the diff is: http://tinyurl.com/62eb88b Why not attach it? What the hell does that URL lead me to? jer
[gentoo-dev] git-2.eclass final review
Hi guys,
as there are no more complaints in kde overlay i would like you to test
your git using live ebuild with this new git-2 eclass and tell me how
you like it.
Also usual review of what already is in is welcomed because i would
really really like to move this thing into main tree.
Cheers
Tom
# Copyright 1999-2011 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Header: $
# @ECLASS: git-2.eclass
# @MAINTAINER:
# Tomas Chvatal scarab...@gentoo.org
# @BLURB: This eclass provides functions for fetch and unpack git repositories
# @DESCRIPTION:
# Eclass for easing maitenance of live ebuilds using git as remote repository.
# Eclass support working with git submodules and branching.
# This eclass support all EAPIs
EXPORT_FUNCTIONS src_unpack
DEPEND=dev-vcs/git
# This static variable is for storing the data in WORKDIR.
# Sometimes we might want to redefine S.
EGIT_SOURCEDIR=${WORKDIR}/${P}
# @FUNCTION: git-2_init_variables
# @DESCRIPTION:
# Internal function initializing all git variables.
# We define it in function scope so user can define
# all the variables before and after inherit.
git-2_init_variables() {
debug-print-function ${FUNCNAME} $@
# @ECLASS-VARIABLE: EGIT_STORE_DIR
# @DESCRIPTION:
# Storage directory for git sources.
: ${EGIT_STORE_DIR:=${PORTAGE_ACTUAL_DISTDIR-${DISTDIR}}/egit-src}
# @ECLASS-VARIABLE: EGIT_HAS_SUBMODULES
# @DESCRIPTION:
# Set this to non-empty value to enable submodule support.
: ${EGIT_HAS_SUBMODULES:=}
# @ECLASS-VARIABLE: EGIT_FETCH_CMD
# @DESCRIPTION:
# Command for cloning the repository.
: ${EGIT_FETCH_CMD:=git clone}
# @ECLASS-VARIABLE: EGIT_UPDATE_CMD
# @DESCRIPTION:
# Git fetch command.
: ${EGIT_UPDATE_CMD:=git pull -f -u}
# @ECLASS-VARIABLE: EGIT_OPTIONS
# @DESCRIPTION:
# This variable value is passed to clone and fetch.
: ${EGIT_OPTIONS:=}
# @ECLASS-VARIABLE: EGIT_MASTER
# @DESCRIPTION:
# Variable for specifying master branch.
# Usefull when upstream don't have master branch.
: ${EGIT_MASTER:=master}
# @ECLASS-VARIABLE: EGIT_REPO_URI
# @DESCRIPTION:
# URI for the repository
# e.g. http://foo, git://bar
#
# Support multiple values:
# EGIT_REPO_URI=git://a/b.git http://c/d.git;
eval X=\$${PN//[-+]/_}_LIVE_REPO
if [[ ${X} = ]]; then
: ${EGIT_REPO_URI:=}
else
EGIT_REPO_URI=${X}
fi
[[ -z ${EGIT_REPO_URI} ]] die EGIT_REPO_URI must have some value.
# @ECLASS-VARIABLE: EVCS_OFFLINE
# @DESCRIPTION:
# Set this variable to a non-empty value to disable the automatic
updating
# of an GIT source tree. This is intended to be set outside the git
source
# tree by users.
: ${EVCS_OFFLINE:=}
# @ECLASS-VARIABLE: EGIT_BRANCH
# @DESCRIPTION:
# Specify the branch we want to check out from the repository
eval X=\$${PN//[-+]/_}_LIVE_BRANCH
if [[ ${X} = ]]; then
: ${EGIT_BRANCH:=${EGIT_MASTER}}
else
EGIT_BRANCH=${X}
fi
# @ECLASS-VARIABLE: EGIT_COMMIT
# @DESCRIPTION:
# Specify commit we want to check out from the repository.
eval X=\$${PN//[-+]/_}_LIVE_COMMIT
if [[ ${X} = ]]; then
: ${EGIT_COMMIT:=${EGIT_BRANCH}}
else
EGIT_COMMIT=${X}
fi
# @ECLASS-VARIABLE: EGIT_REPACK
# @DESCRIPTION:
# Set to non-empty value to repack objects to save disk space. However
this
# can take a REALLY LONG time with VERY big repositories.
: ${EGIT_REPACK:=}
# @ECLASS-VARIABLE: EGIT_PRUNE
# @DESCRIPTION:
# Set to non-empty value to prune loose objects on each fetch. This is
# useful if upstream rewinds and rebases branches often.
: ${EGIT_PRUNE:=}
}
# @FUNCTION: git-2_submodules
# @DESCRIPTION:
# Internal function wrapping the submodule initialisation and update
git-2_submodules() {
debug-print-function ${FUNCNAME} $@
[[ $# -ne 1 ]] die ${FUNCNAME}: requires 1 argument (path)
debug-print ${FUNCNAME}: working in \${1}\
pushd ${1} /dev/null
# for submodules operations we need to be online
if [[ -z ${EVCS_OFFLINE} -n ${EGIT_HAS_SUBMODULES} ]]; then
export GIT_DIR=${EGIT_DIR}
debug-print ${FUNCNAME}: git submodule init
git submodule init \
|| die ${FUNCNAME}: git submodule initialisation
failed
debug-print ${FUNCNAME}: git submodule sync
git submodule sync die ${FUNCNAME}: git submodule sync
failed
debug-print ${FUNCNAME}: git submodule update
Re: [gentoo-dev] git-2.eclass final review
# @BLURB: This eclass provides functions for fetch and unpack git repositories
fetching/unpacking
eval X=\$${PN//[-+]/_}_LIVE_REPO
if [[ ${X} = ]]; then
: ${EGIT_REPO_URI:=}
else
EGIT_REPO_URI=${X}
fi
X needs to be marked local, and could be condensed:
EGIT_REPO_URI=${X:-${EGIT_REPO_URI}}
the PN_LIVE_REPO override documentation is missing
same feedback for all the vars after this too
git-2_fetch $@
missing quotes
-mike
Re: [gentoo-dev] git-2.eclass final review
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Dne 22.3.2011 22:26, Mike Frysinger napsal(a):
# @BLURB: This eclass provides functions for fetch and unpack git
repositories
fetching/unpacking
Yarp fixed.
eval X=\$${PN//[-+]/_}_LIVE_REPO
if [[ ${X} = ]]; then
: ${EGIT_REPO_URI:=}
else
EGIT_REPO_URI=${X}
fi
X needs to be marked local, and could be condensed:
EGIT_REPO_URI=${X:-${EGIT_REPO_URI}}
Hopefully implemented.
the PN_LIVE_REPO override documentation is missing
It is on purpose.
It is semisecret hack that allows you to localy bend live ebuilds
without any need for rewriting it or adding some variables (eg. reason
why we have just one live mesa ebuild and no branches based ones).
If you think this thing should be official we can document it, but i
really really think that people by default should not be aware of this.
same feedback for all the vars after this too
git-2_fetch $@
missing quotes
Fixed
Handy link to gitdiff over gitweb: http://tinyurl.com/6focxa9
Thanks
Tom
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk2JJM0ACgkQHB6c3gNBRYd7fQCffqyoRouirScE2B8npuhEjLEF
Hx4AnRuyyIs5OG3WnRfKgM9itydIpJdE
=Nlbz
-END PGP SIGNATURE-
Re: [gentoo-dev] git-2.eclass final review
2011/3/22 Tomáš Chvátal:
Dne 22.3.2011 22:26, Mike Frysinger napsal(a):
# @BLURB: This eclass provides functions for fetch and unpack git
repositories
fetching/unpacking
Yarp fixed.
well, the fix broke the blurb. it has to be on one line.
# @BLURB: foo
EGIT_BRANCH=${x:-${EGIT_BRANCH:=${EGIT_MASTER}}}
EGIT_COMMIT=${x:-${EGIT_COMMIT:=${EGIT_BRANCH}}}
doesnt make much sense to use := ... it should be :-
[[ $# -ne 1 ]] die ${FUNCNAME}: requires 1 argument (path)
quoting doesnt make much sense ... -ne compares an int, not a string
also, the error msg is a bit vague. it should say ... requires exactly 1 ...
pushd ${1} /dev/null
popd /dev/null
do you really want to silence errors ? normally people only send
stdout to /dev/null because of the echoed dirlist.
seems to come up in every func
git submodule init || die ${FUNCNAME}: git submodule initialisation failed
git submodule sync die ${FUNCNAME}: git submodule sync failed
git submodule update || die ${FUNCNAME}: git submodule update failed
the die strings are abit redundant ... when it fails, the output will
show git submodule init || die, so people can easily figure out the
git submodule init cmd failed
die \${EGIT_BOOTSTRAP}\ is not executable.
i find periods in die messages which are a single sentence to be
noise. but maybe that's just me.
if [[ ${EGIT_COMMIT} != ${EGIT_BRANCH} ]]; then
quoting doesnt matter here since you're using [[...]]
seems to come up in a bunch of places
local save_sandbox_write=${SANDBOX_WRITE}
if [[ ! -d ${EGIT_STORE_DIR} ]] ; then
...
SANDBOX_WRITE=${save_sandbox_write}
fi
might as well have the save done inside the if statement since that's
the only place it's used
rsync -rlpgo . ${EGIT_SOURCEDIR} \
this means you need to have DEPEND=net-misc/rsync. why not just use
`cp -pPR` instead ? i vaguely recall rsync being slower than a
straight cp too ... not much point of doing a rsync when the vast
majority of the time (all the time?) the destination is empty.
git-2_initial_clone()
git-2_update_repo()
shouldnt EGIT_REPO_URI_SELECTED be set to at the start ?
for x in $(git branch |grep -v * ${EGIT_MASTER} |tr '\n' ' '); do
missing space after each pipe
if [[ -n ${EGIT_BOOTSTRAP} ]] ; then
if [[ -f ${EGIT_BOOTSTRAP} ]]; then
seems inconsistent in the whole file ... personally, i prefer the
space before the semicolon in if statements
-mike
Re: [gentoo-dev] git-2.eclass final review
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Dne 23.3.2011 00:08, Mike Frysinger napsal(a):
2011/3/22 Tomáš Chvátal:
Dne 22.3.2011 22:26, Mike Frysinger napsal(a):
# @BLURB: This eclass provides functions for fetch and unpack git
repositories
fetching/unpacking
Yarp fixed.
well, the fix broke the blurb. it has to be on one line.
# @BLURB: foo
Oh damn :P
EGIT_BRANCH=${x:-${EGIT_BRANCH:=${EGIT_MASTER}}}
EGIT_COMMIT=${x:-${EGIT_COMMIT:=${EGIT_BRANCH}}}
doesnt make much sense to use := ... it should be :-
Yeah it should be but still it was just copy/paste and did no harm :)
Altered :)
[[ $# -ne 1 ]] die ${FUNCNAME}: requires 1 argument (path)
quoting doesnt make much sense ... -ne compares an int, not a string
also, the error msg is a bit vague. it should say ... requires exactly 1
...
Altered
pushd ${1} /dev/null
popd /dev/null
do you really want to silence errors ? normally people only send
stdout to /dev/null because of the echoed dirlist.
seems to come up in every func
Yeah that does not matter that much, altered to make stderr visible again.
git submodule init || die ${FUNCNAME}: git submodule initialisation failed
git submodule sync die ${FUNCNAME}: git submodule sync failed
git submodule update || die ${FUNCNAME}: git submodule update failed
the die strings are abit redundant ... when it fails, the output will
show git submodule init || die, so people can easily figure out the
git submodule init cmd failed
Well mostly nobody reads die messages but right here it is pointless.
die \${EGIT_BOOTSTRAP}\ is not executable.
i find periods in die messages which are a single sentence to be
noise. but maybe that's just me.
Does not matter for me either. Since using both i will just stick
withoutdot for dies everywhere :)
if [[ ${EGIT_COMMIT} != ${EGIT_BRANCH} ]]; then
quoting doesnt matter here since you're using [[...]]
seems to come up in a bunch of places
Yeah overquoting is my favorite thing to do during the long and boring
evenings :) removed
local save_sandbox_write=${SANDBOX_WRITE}
if [[ ! -d ${EGIT_STORE_DIR} ]] ; then
...
SANDBOX_WRITE=${save_sandbox_write}
fi
might as well have the save done inside the if statement since that's
the only place it's used
I actualy can't find any good reason why that var is defined at all.
Removed the code.
rsync -rlpgo . ${EGIT_SOURCEDIR} \
this means you need to have DEPEND=net-misc/rsync. why not just use
`cp -pPR` instead ? i vaguely recall rsync being slower than a
straight cp too ... not much point of doing a rsync when the vast
majority of the time (all the time?) the destination is empty.
Good question, i actualy dunno why i picked rsync back then instead of cp.
I would love to use git bare clones (damn submodules) and just use git
to move this crap around. cp -pPR should be equal so lets try with that
and see how much people whine around :)
git-2_initial_clone()
git-2_update_repo()
shouldnt EGIT_REPO_URI_SELECTED be set to at the start ?
Why not :)
And found issue in die string because it said wrong variable there :)
for x in $(git branch |grep -v * ${EGIT_MASTER} |tr '\n' ' '); do
missing space after each pipe
Added
if [[ -n ${EGIT_BOOTSTRAP} ]] ; then
if [[ -f ${EGIT_BOOTSTRAP} ]]; then
seems inconsistent in the whole file ... personally, i prefer the
space before the semicolon in if statements
Yerp i go with the later mostly but sometimes when i copy part statement
i get space into it. Unified.
Thanks for all the points :)
Again the diff is: http://tinyurl.com/62eb88b
Tom
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk2JQgsACgkQHB6c3gNBRYf79QCfV454YtzZD+2m7fOvFHvriDnf
0+4AnjnCA9oCwxPyzWsl8YzAI8ihXdm1
=qdu+
-END PGP SIGNATURE-
