FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 11-06-2010, 08:22 AM
Krzysztof Pawlik
 
Default mercurial.eclass: change clone destination

Hello,

I'm sending this patch for discussion, what it changes? The change is to where
the final clone of repository will be placed, it used to be ${WORKDIR}/${module}
(where module usually is the last component of source URI) to ${WORKDIR}/${P}
(essentially ${S}). This has few effects:
- ebuilds using mercurial.eclass don't need to set S any longer
- mercurial.eclass behaves more like git.eclass
- it breaks all existing ebuilds using this eclass

The last effect is really, really nasty At the same time I think it's worth
fixing this now, not when we have even more ebuilds using mercurial.eclass.
Thankfully the fix to this is trivial: just remove the line where S is being set
(or adjust it to match as is in case of one ebuild in the tree).

Following ebuilds in tree use mercurial.eclass:

dev-python/django-piston/django-piston-9999.ebuild
dev-vcs/mercurial/mercurial-9999.ebuild
media-radio/radiotray/radiotray-9999.ebuild
media-tv/v4l-dvb-hg/v4l-dvb-hg-0.1-r3.ebuild
media-tv/v4l-dvb-hg/v4l-dvb-hg-0.1-r2.ebuild
www-client/dillo/dillo-9999.ebuild
x11-misc/sselp/sselp-9999.ebuild
x11-wm/parti/parti-9999.ebuild

If there are no objections my plan is as follows:
- in a week commit the change to eclass
- fix all above ebuilds
- send a small announcement to gentoo-dev-announcement

Bug: https://bugs.gentoo.org/343993

--
Krzysztof Pawlik <nelchael at gentoo.org> key id: 0xF6A80E46
desktop-misc, java, vim, kernel, python, apache...
Index: mercurial.eclass
================================================== =================
RCS file: /var/cvsroot/gentoo-x86/eclass/mercurial.eclass,v
retrieving revision 1.14
diff -u -r1.14 mercurial.eclass
--- mercurial.eclass 26 Oct 2010 19:04:44 -0000 1.14
+++ mercurial.eclass 6 Nov 2010 09:05:37 -0000
@@ -68,12 +68,14 @@
EHG_OFFLINE="${EHG_OFFLINE:-${ESCM_OFFLINE}}"

# @FUNCTION: mercurial_fetch
-# @USAGE: [repository_uri] [module]
+# @USAGE: [repository_uri] [module] [workdir]
# @DESCRIPTION:
# Clone or update repository.
#
-# If not repository URI is passed it defaults to EHG_REPO_URI, if module is
-# empty it defaults to basename of EHG_REPO_URI.
+# If repository URI is not passed it defaults to EHG_REPO_URI, if module is
+# empty it defaults to basename of EHG_REPO_URI, workdir defaults to P. workdir
+# argument is a directory name under WORKDIR where sources will be available. If
+# you change workdir note that you will need to adjust S to match.
function mercurial_fetch {
debug-print-function ${FUNCNAME} ${*}

@@ -81,6 +83,7 @@
[[ -z "${EHG_REPO_URI}" ]] && die "EHG_REPO_URI is empty"

local module="${2-$(basename "${EHG_REPO_URI}")}"
+ local workdir="${3-${P}}"

# Should be set but blank to prevent using $HOME/.hgrc
export HGRCPATH=
@@ -116,19 +119,19 @@
fi

# Checkout working copy:
- einfo "Creating working directory in ${WORKDIR}/${module} (target revision: ${EHG_REVISION})"
+ einfo "Creating working directory in ${WORKDIR}/${workdir} (target revision: ${EHG_REVISION})"
hg clone
${EHG_QUIET_CMD_OPT}
--rev="${EHG_REVISION}"
"${EHG_STORE_DIR}/${EHG_PROJECT}/${module}"
- "${WORKDIR}/${module}" || die "hg clone failed"
+ "${WORKDIR}/${workdir}" || die "hg clone failed"
# An exact revision helps a lot for testing purposes, so have some output...
# id num branch
# fd6e32d61721 6276 default
- local HG_REVDATA=($(hg identify -b -i "${WORKDIR}/${module}"))
+ local HG_REVDATA=($(hg identify -b -i "${WORKDIR}/${workdir}"))
export HG_REV_ID=${HG_REVDATA[0]}
local HG_REV_BRANCH=${HG_REVDATA[1]}
- einfo "Work directory: ${WORKDIR}/${module} global id: ${HG_REV_ID} branch: ${HG_REV_BRANCH}"
+ einfo "Work directory: ${WORKDIR}/${workdir} global id: ${HG_REV_ID} branch: ${HG_REV_BRANCH}"
}

# @FUNCTION: mercurial_src_unpack
 
Old 11-07-2010, 01:30 AM
Donnie Berkholz
 
Default mercurial.eclass: change clone destination

On 10:22 Sat 06 Nov , Krzysztof Pawlik wrote:
> I'm sending this patch for discussion, what it changes? The change is
> to where the final clone of repository will be placed, it used to be
> ${WORKDIR}/${module} (where module usually is the last component of
> source URI) to ${WORKDIR}/${P} (essentially ${S}). This has few
> effects:
> - ebuilds using mercurial.eclass don't need to set S any longer
> - mercurial.eclass behaves more like git.eclass
> - it breaks all existing ebuilds using this eclass
>
> The last effect is really, really nasty At the same time I think
> it's worth fixing this now, not when we have even more ebuilds using
> mercurial.eclass. Thankfully the fix to this is trivial: just remove
> the line where S is being set (or adjust it to match as is in case of
> one ebuild in the tree).

Krzysztof,

I see you've said you're breaking all these ebuilds but I don't see any
rationale for the change, at least in this email. Perhaps you could
explain why this level of breakage is necessary?

--
Thanks,
Donnie

Donnie Berkholz
Sr. Developer, Gentoo Linux
Blog: http://dberkholz.wordpress.com
 
Old 11-07-2010, 01:40 AM
Donnie Berkholz
 
Default mercurial.eclass: change clone destination

On 21:30 Sat 06 Nov , Donnie Berkholz wrote:
> On 10:22 Sat 06 Nov , Krzysztof Pawlik wrote:
> > I'm sending this patch for discussion, what it changes? The change is
> > to where the final clone of repository will be placed, it used to be
> > ${WORKDIR}/${module} (where module usually is the last component of
> > source URI) to ${WORKDIR}/${P} (essentially ${S}). This has few
> > effects:
> > - ebuilds using mercurial.eclass don't need to set S any longer
> > - mercurial.eclass behaves more like git.eclass
> > - it breaks all existing ebuilds using this eclass
> >
> > The last effect is really, really nasty At the same time I think
> > it's worth fixing this now, not when we have even more ebuilds using
> > mercurial.eclass. Thankfully the fix to this is trivial: just remove
> > the line where S is being set (or adjust it to match as is in case of
> > one ebuild in the tree).
>
> Krzysztof,
>
> I see you've said you're breaking all these ebuilds but I don't see any
> rationale for the change, at least in this email. Perhaps you could
> explain why this level of breakage is necessary?

I read it more closely and realized I was a little confused by the way
you listed all the bullet points mixing together benefits and problems.

So I'll try again: if you really want to do this change, you might want
to consider adding a mercurial-2.eclass instead. Eclasses of this nature
(svn, git, hg, etc) tend to be broadly used outside the tree as well as
within, so breaking backwards compatibility can be a real problem. A new
versioned eclass allows for a much more gradual transition.

--
Thanks,
Donnie

Donnie Berkholz
Sr. Developer, Gentoo Linux
Blog: http://dberkholz.wordpress.com
 
Old 11-07-2010, 11:07 AM
Mike Auty
 
Default mercurial.eclass: change clone destination

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

On 07/11/10 02:40, Donnie Berkholz wrote:
> I read it more closely and realized I was a little confused by the way
> you listed all the bullet points mixing together benefits and problems.
>
> So I'll try again: if you really want to do this change, you might want
> to consider adding a mercurial-2.eclass instead. Eclasses of this nature
> (svn, git, hg, etc) tend to be broadly used outside the tree as well as
> within, so breaking backwards compatibility can be a real problem. A new
> versioned eclass allows for a much more gradual transition.
>

I've only just jumped into the conversation, but the obvious question
is, why not just use ${S} as the location of the working directory
(rather than "${WORKDIR}/${workdir}")? That way, if it's set old
ebuilds still work, and if it's not set, new ebuilds get the benefit of
using ${S}? I can only see a problem with this if there's somewhere
that the value of the working directory needs to be known before any of
the phases...

Mike 5
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.16 (GNU/Linux)

iEYEARECAAYFAkzWloYACgkQu7rWomwgFXosEACgiFBLbFABwe QR0JrZqprBxdkT
10UAoJVESt/2T3Y1AXkBXu7qYPY4NBf2
=ULZu
-----END PGP SIGNATURE-----
 
Old 11-07-2010, 01:42 PM
Petteri Räty
 
Default mercurial.eclass: change clone destination

On 11/06/2010 11:22 AM, Krzysztof Pawlik wrote:
> Hello,
>
> I'm sending this patch for discussion, what it changes? The change is to where
> the final clone of repository will be placed, it used to be ${WORKDIR}/${module}
> (where module usually is the last component of source URI) to ${WORKDIR}/${P}
> (essentially ${S}). This has few effects:
> - ebuilds using mercurial.eclass don't need to set S any longer
> - mercurial.eclass behaves more like git.eclass
> - it breaks all existing ebuilds using this eclass
>

Which means that the doing the chance is not allowed as eclasses must
remain backwards compatible.

Regards,
Petteri
 
Old 11-07-2010, 10:08 PM
Krzysztof Pawlik
 
Default mercurial.eclass: change clone destination

On 11/07/10 13:07, Mike Auty wrote:
> On 07/11/10 02:40, Donnie Berkholz wrote:
>> I read it more closely and realized I was a little confused by the way
>> you listed all the bullet points mixing together benefits and problems.
>
>> So I'll try again: if you really want to do this change, you might want
>> to consider adding a mercurial-2.eclass instead. Eclasses of this nature
>> (svn, git, hg, etc) tend to be broadly used outside the tree as well as
>> within, so breaking backwards compatibility can be a real problem. A new
>> versioned eclass allows for a much more gradual transition.
>
>
> I've only just jumped into the conversation, but the obvious question
> is, why not just use ${S} as the location of the working directory
> (rather than "${WORKDIR}/${workdir}")? That way, if it's set old
> ebuilds still work, and if it's not set, new ebuilds get the benefit of
> using ${S}? I can only see a problem with this if there's somewhere
> that the value of the working directory needs to be known before any of
> the phases...

Hm.. good idea I'm attaching modified patch that uses ${S} by default, so it
will improve the situation and at the same time it won't break existing ebuilds.
Thanks Mike for this suggestion.

--
Krzysztof Pawlik <nelchael at gentoo.org> key id: 0xF6A80E46
desktop-misc, java, vim, kernel, python, apache...
Index: mercurial.eclass
================================================== =================
RCS file: /var/cvsroot/gentoo-x86/eclass/mercurial.eclass,v
retrieving revision 1.14
diff -u -r1.14 mercurial.eclass
--- mercurial.eclass 26 Oct 2010 19:04:44 -0000 1.14
+++ mercurial.eclass 7 Nov 2010 23:05:22 -0000
@@ -68,12 +68,12 @@
EHG_OFFLINE="${EHG_OFFLINE:-${ESCM_OFFLINE}}"

# @FUNCTION: mercurial_fetch
-# @USAGE: [repository_uri] [module]
+# @USAGE: [repository_uri] [module] [sourcedir]
# @DESCRIPTION:
# Clone or update repository.
#
-# If not repository URI is passed it defaults to EHG_REPO_URI, if module is
-# empty it defaults to basename of EHG_REPO_URI.
+# If repository URI is not passed it defaults to EHG_REPO_URI, if module is
+# empty it defaults to basename of EHG_REPO_URI, sourcedir defaults to S.
function mercurial_fetch {
debug-print-function ${FUNCNAME} ${*}

@@ -81,6 +81,7 @@
[[ -z "${EHG_REPO_URI}" ]] && die "EHG_REPO_URI is empty"

local module="${2-$(basename "${EHG_REPO_URI}")}"
+ local sourcedir="${3-${S}}"

# Should be set but blank to prevent using $HOME/.hgrc
export HGRCPATH=
@@ -116,19 +117,19 @@
fi

# Checkout working copy:
- einfo "Creating working directory in ${WORKDIR}/${module} (target revision: ${EHG_REVISION})"
+ einfo "Creating working directory in ${sourcedir} (target revision: ${EHG_REVISION})"
hg clone
${EHG_QUIET_CMD_OPT}
--rev="${EHG_REVISION}"
"${EHG_STORE_DIR}/${EHG_PROJECT}/${module}"
- "${WORKDIR}/${module}" || die "hg clone failed"
+ "${sourcedir}" || die "hg clone failed"
# An exact revision helps a lot for testing purposes, so have some output...
# id num branch
# fd6e32d61721 6276 default
- local HG_REVDATA=($(hg identify -b -i "${WORKDIR}/${module}"))
+ local HG_REVDATA=($(hg identify -b -i "${sourcedir}"))
export HG_REV_ID=${HG_REVDATA[0]}
local HG_REV_BRANCH=${HG_REVDATA[1]}
- einfo "Work directory: ${WORKDIR}/${module} global id: ${HG_REV_ID} branch: ${HG_REV_BRANCH}"
+ einfo "Work directory: ${sourcedir} global id: ${HG_REV_ID} branch: ${HG_REV_BRANCH}"
}

# @FUNCTION: mercurial_src_unpack
 
Old 11-08-2010, 03:17 AM
Donnie Berkholz
 
Default mercurial.eclass: change clone destination

On 16:42 Sun 07 Nov , Petteri Räty wrote:
> On 11/06/2010 11:22 AM, Krzysztof Pawlik wrote:
> > Hello,
> >
> > I'm sending this patch for discussion, what it changes? The change is to where
> > the final clone of repository will be placed, it used to be ${WORKDIR}/${module}
> > (where module usually is the last component of source URI) to ${WORKDIR}/${P}
> > (essentially ${S}). This has few effects:
> > - ebuilds using mercurial.eclass don't need to set S any longer
> > - mercurial.eclass behaves more like git.eclass
> > - it breaks all existing ebuilds using this eclass
>
> Which means that the doing the chance is not allowed as eclasses must
> remain backwards compatible.

Is that really still the case now that full environments have been saved
for a number of years? Clearly breaking things is unacceptable, but I
could envision someone simultaneously updating the eclass and all
consumers.

Perhaps you could point me to the appropriate documentation, if any
exists.

--
Thanks,
Donnie

Donnie Berkholz
Sr. Developer, Gentoo Linux
Blog: http://dberkholz.wordpress.com
 
Old 11-08-2010, 07:05 PM
Petteri Räty
 
Default mercurial.eclass: change clone destination

On 11/08/2010 06:17 AM, Donnie Berkholz wrote:
> On 16:42 Sun 07 Nov , Petteri Räty wrote:
>> On 11/06/2010 11:22 AM, Krzysztof Pawlik wrote:
>>> Hello,
>>>
>>> I'm sending this patch for discussion, what it changes? The change is to where
>>> the final clone of repository will be placed, it used to be ${WORKDIR}/${module}
>>> (where module usually is the last component of source URI) to ${WORKDIR}/${P}
>>> (essentially ${S}). This has few effects:
>>> - ebuilds using mercurial.eclass don't need to set S any longer
>>> - mercurial.eclass behaves more like git.eclass
>>> - it breaks all existing ebuilds using this eclass
>>
>> Which means that the doing the chance is not allowed as eclasses must
>> remain backwards compatible.
>
> Is that really still the case now that full environments have been saved
> for a number of years? Clearly breaking things is unacceptable, but I
> could envision someone simultaneously updating the eclass and all
> consumers.
>

There's no full environment saved before the package is installed and I
don't see why we should break overlays.

Regards,
Petteri
 
Old 11-10-2010, 04:16 PM
William Hubbs
 
Default mercurial.eclass: change clone destination

On Mon, Nov 08, 2010 at 10:05:17PM +0200, Petteri R??ty wrote:
> On 11/08/2010 06:17 AM, Donnie Berkholz wrote:
> > On 16:42 Sun 07 Nov , Petteri R??ty wrote:
> >> On 11/06/2010 11:22 AM, Krzysztof Pawlik wrote:
> >>> Hello,
> >>>
> >>> I'm sending this patch for discussion, what it changes? The change is to where
> >>> the final clone of repository will be placed, it used to be ${WORKDIR}/${module}
> >>> (where module usually is the last component of source URI) to ${WORKDIR}/${P}
> >>> (essentially ${S}). This has few effects:
> >>> - ebuilds using mercurial.eclass don't need to set S any longer
> >>> - mercurial.eclass behaves more like git.eclass
> >>> - it breaks all existing ebuilds using this eclass
> >>
> >> Which means that the doing the chance is not allowed as eclasses must
> >> remain backwards compatible.
> >
> > Is that really still the case now that full environments have been saved
> > for a number of years? Clearly breaking things is unacceptable, but I
> > could envision someone simultaneously updating the eclass and all
> > consumers.
> >
>
> There's no full environment saved before the package is installed and I
> don't see why we should break overlays.

I didn't think we had to care about overlays since they aren't the main
tree? After all, how can we be sure what is happening in all overlays
out there?

I could see this, like Donnie says, if he wasn't going to fix all of the
ebuilds. However I don't see a problem with it since he said he is
going to fix all of the ebuilds.

William
 
Old 11-10-2010, 07:56 PM
Krzysztof Pawlik
 
Default mercurial.eclass: change clone destination

On 11/10/10 18:16, William Hubbs wrote:
> On Mon, Nov 08, 2010 at 10:05:17PM +0200, Petteri R??ty wrote:
>> On 11/08/2010 06:17 AM, Donnie Berkholz wrote:
>>> On 16:42 Sun 07 Nov , Petteri R??ty wrote:
>>>> On 11/06/2010 11:22 AM, Krzysztof Pawlik wrote:
>>>>> Hello,
>>>>>
>>>>> I'm sending this patch for discussion, what it changes? The change is to where
>>>>> the final clone of repository will be placed, it used to be ${WORKDIR}/${module}
>>>>> (where module usually is the last component of source URI) to ${WORKDIR}/${P}
>>>>> (essentially ${S}). This has few effects:
>>>>> - ebuilds using mercurial.eclass don't need to set S any longer
>>>>> - mercurial.eclass behaves more like git.eclass
>>>>> - it breaks all existing ebuilds using this eclass
>>>>
>>>> Which means that the doing the chance is not allowed as eclasses must
>>>> remain backwards compatible.
>>>
>>> Is that really still the case now that full environments have been saved
>>> for a number of years? Clearly breaking things is unacceptable, but I
>>> could envision someone simultaneously updating the eclass and all
>>> consumers.
>>>
>>
>> There's no full environment saved before the package is installed and I
>> don't see why we should break overlays.
>
> I didn't think we had to care about overlays since they aren't the main
> tree? After all, how can we be sure what is happening in all overlays
> out there?

Annoying a lot of users is not a good idea anyway

> I could see this, like Donnie says, if he wasn't going to fix all of the
> ebuilds. However I don't see a problem with it since he said he is
> going to fix all of the ebuilds.

Yes, but let's drop the issue - last version of patch doesn't break anything -
it uses ${S} which means it will work equally well for old ebuilds and also will
achieve what I want (to respect ${S}).

--
Krzysztof Pawlik <nelchael at gentoo.org> key id: 0xF6A80E46
desktop-misc, java, vim, kernel, python, apache...
 

Thread Tools




All times are GMT. The time now is 07:37 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright ©2007 - 2008, www.linux-archive.org