Some open issues

Some open issues

[Richard Purdie]

Hi Guys,

I have a few issues with the way certain things have happened recently.

1. The FILE_PR change.

This was mentioned in an email on Wednesday with the title "[oe] [RFC]
Enable --hash-style=both for all recent gcc4 targets" at 9am. At 8pm we
have "I decided to land now as PRs are changing all the time and keeping
up with things is pretty hard...". This is not in keeping with the major
changes policy we agreed by any stretch of the imagination.

This change breaks compatibility with everyone's overlays and creates
the nightmare scenario of external OE "branches" being forced into the
change or forever being unable to sync (Openmoko and Poky spring to
mind).

What is most annoying is that given a bit longer I think we could have
done something that would have meant this was unnecessary, specifically
inserted the revision into the package at package_*.bbclass time where
we can manipulated PR as needed. This combined with a staging ABI change
would have been all that was needed. If staging ABI isn't enough, we can
insert the modified PR into STAMPS instead of the real PR or some other
magic. My point is that there are better options than FILE_PR, it just
needs some thought. The fact the testing branch had so many merge issues
should have meant a better idea was sought, not that is should be
committed ASAP.

2. The Git conversion including the BKCVS data.

I'd made it quite clear this should have been a tree graft and this
wasn't done so we're now stuck with broken history :(. This is pretty
frustrating since I'd repeatedly said not to include it and went to the
effort of gathering my conversion data and sending it to Jan who then
didn't realise what I meant by graft (though no fault of his own).

3. Merging Bitbake into OE

People are saying things like "Might be a chance to reconsider
maintaining BitBake in the OE repository.". Could people please talk to
the bitbake maintainer about things like this *before* encouraging it in
public. If we need a release lets make one (which seems to be the real
problem).

4. Bitbake changes

These should go to the bitbake list as well as the OE list and should be
discussed. I've raised issues with patches which have been ignored and
these patches have now just need committed. I'm not happy about the
process that was used :(. I know people have various commitments but we
need to try and stick to some kind of process for this kind of thing.


Where from here?

I'd actually like to a strong line on this and suggest we revert the
FILE_PR change since its badly thought out and also that we consider
redoing the git conversion ASAP and the replaying the recent commits
before its too late to get rid of the corruption in there. This is
probably going to have to go to a core team vote since its a pretty big
change to suggest but opinions are welcome.

Richard

Re: Some open issues

[Holger Freyther]

On Saturday 18 October 2008 11:03:29 Richard Purdie wrote:
> Hi Guys,
>
> I have a few issues with the way certain things have happened recently.
>
> 1. The FILE_PR change.
>
> This was mentioned in an email on Wednesday with the title "[oe] [RFC]
> Enable --hash-style=both for all recent gcc4 targets" at 9am. At 8pm we
> have "I decided to land now as PRs are changing all the time and keeping
> up with things is pretty hard...". This is not in keeping with the major
> changes policy we agreed by any stretch of the imagination.

Sorry about that.


> This change breaks compatibility with everyone's overlays and creates
> the nightmare scenario of external OE "branches" being forced into the
> change or forever being unable to sync (Openmoko and Poky spring to
> mind).

Yes, this creates an extra burden for the people that need to keep that in 
sync.


> What is most annoying is that given a bit longer I think we could have
> done something that would have meant this was unnecessary, specifically
> inserted the revision into the package at package_*.bbclass time where
> we can manipulated PR as needed. This combined with a staging ABI change
> would have been all that was needed. If staging ABI isn't enough, we can
> insert the modified PR into STAMPS instead of the real PR or some other
> magic. My point is that there are better options than FILE_PR, it just
> needs some thought. The fact the testing branch had so many merge issues
> should have meant a better idea was sought, not that is should be
> committed ASAP.

I don't agree with this. This means package names will not match with the on 
dir directory name. For me this was the strongest argument against doing it 
automatically in the packaging bbclass. I really don't like the idea that if 
I bump the DISTRO_PR the existing build directories will be recycled and 
packages will be rebuild in there. That mostly defeats the purpose of a 
deterministic tool. And I really don't like the idea of adding more python 
black magic that is mangling PR... :)



>
> 2. The Git conversion including the BKCVS data.
>
> I'd made it quite clear this should have been a tree graft and this
> wasn't done so we're now stuck with broken history :(. This is pretty
> frustrating since I'd repeatedly said not to include it and went to the
> effort of gathering my conversion data and sending it to Jan who then
> didn't realise what I meant by graft (though no fault of his own).

Should we start over from scratch and rebase what we have already added? Will 
this be the _last_ time we consider it?



> 4. Bitbake changes
>
> These should go to the bitbake list as well as the OE list and should be
> discussed. I've raised issues with patches which have been ignored and
> these patches have now just need committed. I'm not happy about the
> process that was used :(. I know people have various commitments but we
> need to try and stick to some kind of process for this kind of thing.

???? Things have been ignored... three times...


>
>
> Where from here?
>
> I'd actually like to a strong line on this and suggest we revert the
> FILE_PR change since its badly thought out and also that we consider
> redoing the git conversion ASAP and the replaying the recent commits
> before its too late to get rid of the corruption in there. This is
> probably going to have to go to a core team vote since its a pretty big
> change to suggest but opinions are welcome.

Do whatever you want. I think FILE_PR is the right thing to do, feel free to 
invest the time to redo the conversion, the scripts are public and Jan might 
be able to upload the "source" for git-fast-import, please also coordinate 
the git-rebase (we will lose merges but that seems okay, but I did merge the 
FILE_PR changes on purpose for ease of reverting), make sure no one is 
merging old history with new history (this was not a big problem with the 
trial but will be one now)...

FILE_PR and my requirements Re: Some open issues

[Holger Freyther]

On Saturday 18 October 2008 14:32:37 Holger Freyther wrote:

More in depth:

Issues with the current approach:
	- OE => poky is more hard to sync as one needs to watch PR
	- We need to make sure that people do not introduce PR but use FILE_PR

Alternatives:
	- Python black magic(*)
	- A new set of variables + bitbake change

Alternative Python black magic:
	- I strongly oppose any new transparent python magic. I constantly see people 
confused by what we do behind the scenes. I want that PF (so WORKDIR) 
containts the PN, PV, 'PR' of the resulting package. It can not be that you 
have a PR r10.1 as package but only a r10 in your work directory.
	- If I'm not mistaken about the above I will not be happy with such a 
solution


Alternative a new set of variables:
	- Revert to make PR as it was before. This adds legacy...
	- Introduce PKG_PR which is the composition of PR and DISTRO_PR
	- Change PF to be PN-PV-PR (this influences the stamps)
	- Change Bitbake to cache PKG_PR and prefer it over PR.


comments?

Re: FILE_PR and my requirements Re: Some open issues

[Holger Freyther]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

On Saturday 18 October 2008 15:20:05 Holger Freyther wrote:
> On Saturday 18 October 2008 14:32:37 Holger Freyther wrote:
>
> More in depth:
- We need to make sure that people do not introduce PR but use FILE_PR

Oops I missed my requirements:
	- Easy to understand what is going on
	- Package name and Workdir matching with each other
	- Full rebuild on DISTRO_PR bump
	- Not reusing the same build directory



> Alternative a new set of variables:
> 	- Revert to make PR as it was before. This adds legacy...
> 	- Introduce PKG_PR which is the composition of PR and DISTRO_PR
> 	- Change PF to be PN-PV-PR (this influences the stamps)
> 	- Change Bitbake to cache PKG_PR and prefer it over PR.

And here two patches. The amount of changes is a lot smaller. The downside is 
it is not as easy to understand as changin PR to FILE_PR (think of the people 
merging OE into their private trees that know little about the classes...). 
Anyway this is something we can talk about..



[-- Attachment #2: bitbake-pkg_pr.diff --]
[-- Type: text/x-diff, Size: 1236 bytes --]

diff --git a/lib/bb/cache.py b/lib/bb/cache.py
index fe38ea0..58b9e49 100644
--- a/lib/bb/cache.py
+++ b/lib/bb/cache.py
@@ -39,7 +39,7 @@ except ImportError:
     import pickle
     bb.msg.note(1, bb.msg.domain.Cache, "Importing cPickle failed. Falling back to a very slow implementation.")
 
-__cache_version__ = "129"
+__cache_version__ = "130"
 
 class Cache:
     """
@@ -286,6 +286,7 @@ class Cache:
         pe       = self.getVar('PE', file_name, True) or "0"
         pv       = self.getVar('PV', file_name, True)
         pr       = self.getVar('PR', file_name, True)
+        pkg_pr   = self.getVar('PKG_PR', file_name, True) or pr
         dp       = int(self.getVar('DEFAULT_PREFERENCE', file_name, True) or "0")
         depends   = bb.utils.explode_deps(self.getVar("DEPENDS", file_name, True) or "")
         packages  = (self.getVar('PACKAGES', file_name, True) or "").split()
@@ -303,7 +304,7 @@ class Cache:
 
         # build FileName to PackageName lookup table
         cacheData.pkg_fn[file_name] = pn
-        cacheData.pkg_pepvpr[file_name] = (pe,pv,pr)
+        cacheData.pkg_pepvpr[file_name] = (pe,pv,pkg_pr)
         cacheData.pkg_dp[file_name] = dp
 
         provides = [pn]

[-- Attachment #3: oe-pkg-pr.diff --]
[-- Type: text/x-diff, Size: 11279 bytes --]

diff --git a/classes/kernel.bbclass b/classes/kernel.bbclass
index 266a89d..3e215f8 100644
--- a/classes/kernel.bbclass
+++ b/classes/kernel.bbclass
@@ -468,7 +468,7 @@ do_sizecheck() {
 
 addtask sizecheck before do_install after do_compile
 
-KERNEL_IMAGE_BASE_NAME ?= "${KERNEL_IMAGETYPE}-${PV}-${PR}-${MACHINE}"
+KERNEL_IMAGE_BASE_NAME ?= "${KERNEL_IMAGETYPE}-${PV}-${PKG_PR}-${MACHINE}"
 KERNEL_IMAGE_SYMLINK_NAME ?= "${KERNEL_IMAGETYPE}-${MACHINE}"
 
 do_deploy() {
@@ -477,7 +477,7 @@ do_deploy() {
 	package_stagefile_shell ${DEPLOY_DIR_IMAGE}/${KERNEL_IMAGE_BASE_NAME}.bin
 
 	if [ -d "${D}/lib" ]; then
-	tar -cvzf ${DEPLOY_DIR_IMAGE}/modules-${PV}-${PR}-${MACHINE}.tgz -C ${D} lib
+	tar -cvzf ${DEPLOY_DIR_IMAGE}/modules-${PV}-${PKG_PR}-${MACHINE}.tgz -C ${D} lib
 	fi
 
 	if test "x${KERNEL_IMAGETYPE}" = "xuImage" ; then 
diff --git a/classes/oestats-client.bbclass b/classes/oestats-client.bbclass
index 6d348d3..5e3016f 100644
--- a/classes/oestats-client.bbclass
+++ b/classes/oestats-client.bbclass
@@ -144,7 +144,7 @@ def oestats_task(server, d, task, status):
 		'build': id,
 		'package': bb.data.getVar('PN', d, True),
 		'version': bb.data.getVar('PV', d, True),
-		'revision': bb.data.getVar('PR', d, True),
+		'revision': bb.data.getVar('PKG_PR', d, True),
 		'depends': bb.data.getVar('DEPENDS', d, True),
 		'task': task,
 		'status': status,
diff --git a/classes/package.bbclass b/classes/package.bbclass
index a12bfb0..10d1c18 100644
--- a/classes/package.bbclass
+++ b/classes/package.bbclass
@@ -513,7 +513,7 @@ python emit_pkgdata() {
 		subdata_file = pkgdatadir + "/runtime/%s" % pkg
 		sf = open(subdata_file, 'w')
 		write_if_exists(sf, pkg, 'PN')
-		write_if_exists(sf, pkg, 'PR')
+		write_if_exists(sf, pkg, 'PKG_PR')
 		write_if_exists(sf, pkg, 'DESCRIPTION')
 		write_if_exists(sf, pkg, 'RDEPENDS')
 		write_if_exists(sf, pkg, 'RPROVIDES')
diff --git a/classes/package_deb.bbclass b/classes/package_deb.bbclass
index e59d194..84a1477 100644
--- a/classes/package_deb.bbclass
+++ b/classes/package_deb.bbclass
@@ -131,7 +131,7 @@ python do_package_deb () {
             pass
         if not g and bb.data.getVar('ALLOW_EMPTY', localdata) != "1":
             from bb import note
-            note("Not creating empty archive for %s-%s-%s" % (pkg, bb.data.getVar('PV', localdata, 1), bb.data.getVar('PR', localdata, 1)))
+            note("Not creating empty archive for %s-%s-%s" % (pkg, bb.data.getVar('PV', localdata, 1), bb.data.getVar('PKG_PR', localdata, 1)))
             bb.utils.unlockfile(lf)
             continue
 
@@ -149,9 +149,9 @@ python do_package_deb () {
         fields = []
         pe = bb.data.getVar('PE', d, 1)
         if pe and int(pe) > 0:
-            fields.append(["Version: %s:%s-%s\n", ['PE', 'PV', 'PR']])
+            fields.append(["Version: %s:%s-%s\n", ['PE', 'PV', 'PKG_PR']])
         else:
-            fields.append(["Version: %s-%s\n", ['PV', 'PR']])
+            fields.append(["Version: %s-%s\n", ['PV', 'PKG_PR']])
         fields.append(["Description: %s\n", ['DESCRIPTION']])
         fields.append(["Section: %s\n", ['SECTION']])
         fields.append(["Priority: %s\n", ['PRIORITY']])
diff --git a/classes/package_ipk.bbclass b/classes/package_ipk.bbclass
index f05b744..a195e75 100644
--- a/classes/package_ipk.bbclass
+++ b/classes/package_ipk.bbclass
@@ -178,7 +178,7 @@ python do_package_ipk () {
 			pass
 		if not g and bb.data.getVar('ALLOW_EMPTY', localdata) != "1":
 			from bb import note
-			note("Not creating empty archive for %s-%s-%s" % (pkg, bb.data.getVar('PV', localdata, 1), bb.data.getVar('PR', localdata, 1)))
+			note("Not creating empty archive for %s-%s-%s" % (pkg, bb.data.getVar('PV', localdata, 1), bb.data.getVar('PKG_PR', localdata, 1)))
 			bb.utils.unlockfile(lf)
 			continue
 
@@ -193,9 +193,9 @@ python do_package_ipk () {
 		fields = []
 		pe = bb.data.getVar('PE', d, 1)
 		if pe and int(pe) > 0:
-			fields.append(["Version: %s:%s-%s\n", ['PE', 'PV', 'PR']])
+			fields.append(["Version: %s:%s-%s\n", ['PE', 'PV', 'PKG_PR']])
 		else:
-			fields.append(["Version: %s-%s\n", ['PV', 'PR']])
+			fields.append(["Version: %s-%s\n", ['PV', 'PKG_PR']])
 		fields.append(["Description: %s\n", ['DESCRIPTION']])
 		fields.append(["Section: %s\n", ['SECTION']])
 		fields.append(["Priority: %s\n", ['PRIORITY']])
diff --git a/classes/package_rpm.bbclass b/classes/package_rpm.bbclass
index 6713f8f..08f9852 100644
--- a/classes/package_rpm.bbclass
+++ b/classes/package_rpm.bbclass
@@ -10,7 +10,7 @@ python write_specfile() {
 	out_vartranslate = {
 		"PKG": "Name",
 		"PV": "Version",
-		"PR": "Release",
+		"PKG_PR": "Release",
 		"DESCRIPTION": "%description",
 		"ROOT": "BuildRoot",
 		"LICENSE": "License",
@@ -41,7 +41,7 @@ python write_specfile() {
 			pass
 	if not files:
 		from bb import note
-		note("Not creating empty archive for %s-%s-%s" % (bb.data.getVar('PKG',d, 1), bb.data.getVar('PV', d, 1), bb.data.getVar('PR', d, 1)))
+		note("Not creating empty archive for %s-%s-%s" % (bb.data.getVar('PKG',d, 1), bb.data.getVar('PV', d, 1), bb.data.getVar('PKG_PR', d, 1)))
 		return
 
 	# output .spec using this metadata store
@@ -79,8 +79,8 @@ python write_specfile() {
 	bb.build.exec_func('BUILDSPEC', d)
 
 	# move the rpm into the pkgoutdir
-	rpm = bb.data.expand('${RPMBUILDPATH}/RPMS/${TARGET_ARCH}/${PKG}-${PV}-${PR}.${TARGET_ARCH}.rpm', d)
-	outrpm = bb.data.expand('${DEPLOY_DIR_RPM}/${PKG}-${PV}-${PR}.${TARGET_ARCH}.rpm', d)
+	rpm = bb.data.expand('${RPMBUILDPATH}/RPMS/${TARGET_ARCH}/${PKG}-${PV}-${PKG_PR}.${TARGET_ARCH}.rpm', d)
+	outrpm = bb.data.expand('${DEPLOY_DIR_RPM}/${PKG}-${PV}-${PKG_PR}.${TARGET_ARCH}.rpm', d)
 	bb.movefile(rpm, outrpm)
 }
 
diff --git a/classes/package_tar.bbclass b/classes/package_tar.bbclass
index 876cec6..dfe9c34 100644
--- a/classes/package_tar.bbclass
+++ b/classes/package_tar.bbclass
@@ -5,7 +5,7 @@ IMAGE_PKGTYPE ?= "tar"
 python package_tar_fn () {
 	import os
 	from bb import data
-	fn = os.path.join(bb.data.getVar('DEPLOY_DIR_TAR', d), "%s-%s-%s.tar.gz" % (bb.data.getVar('PKG', d), bb.data.getVar('PV', d), bb.data.getVar('PR', d)))
+	fn = os.path.join(bb.data.getVar('DEPLOY_DIR_TAR', d), "%s-%s-%s.tar.gz" % (bb.data.getVar('PKG', d), bb.data.getVar('PV', d), bb.data.getVar('PKG_PR', d)))
 	fn = bb.data.expand(fn, d)
 	bb.data.setVar('PKGFN', fn, d)
 }
@@ -86,7 +86,7 @@ python do_package_tar () {
 		os.chdir(root)
 		from glob import glob
 		if not glob('*'):
-			bb.note("Not creating empty archive for %s-%s-%s" % (pkg, bb.data.getVar('PV', localdata, 1), bb.data.getVar('PR', localdata, 1)))
+			bb.note("Not creating empty archive for %s-%s-%s" % (pkg, bb.data.getVar('PV', localdata, 1), bb.data.getVar('PKG_PR', localdata, 1)))
 			continue
 		ret = os.system("tar -czf %s %s" % (tarfn, '.'))
 		if ret != 0:
diff --git a/classes/packaged-staging.bbclass b/classes/packaged-staging.bbclass
index ed94790..2ddbc89 100644
--- a/classes/packaged-staging.bbclass
+++ b/classes/packaged-staging.bbclass
@@ -11,7 +11,7 @@
 #
 # bitbake.conf set PSTAGING_ACTIVE = "0", this class sets to "1" if we're active
 # 
-PSTAGE_PKGVERSION = "${PV}-${PR}"
+PSTAGE_PKGVERSION = "${PV}-${PKG_PR}"
 PSTAGE_PKGARCH    = "${BUILD_SYS}"
 PSTAGE_EXTRAPATH  ?= ""
 PSTAGE_PKGPATH    = "${DISTRO}${PSTAGE_EXTRAPATH}"
@@ -369,9 +369,9 @@ python do_package_stage () {
             arch = bb.data.getVar('PACKAGE_ARCH_%s' % pkg, d, 1)
             if not arch:
                 arch = bb.data.getVar('PACKAGE_ARCH', d, 1)
-            pr = bb.data.getVar('PR_%s' % pkg, d, 1)
+            pr = bb.data.getVar('PKG_PR_%s' % pkg, d, 1)
             if not pr:
-                pr = bb.data.getVar('PR', d, 1)
+                pr = bb.data.getVar('PKG_PR', d, 1)
             if not packaged(pkg, d):
                 continue
             if bb.data.inherits_class('package_ipk', d):
diff --git a/classes/sourcepkg.bbclass b/classes/sourcepkg.bbclass
index bbc9f18..f68a752 100644
--- a/classes/sourcepkg.bbclass
+++ b/classes/sourcepkg.bbclass
@@ -63,7 +63,7 @@ python sourcepkg_do_dumpdata() {
 	distro = bb.data.getVar('DISTRO', d, 1)
 	s_tree = get_src_tree(d)
 	openembeddeddir = os.path.join(workdir, s_tree, distro)
-	dumpfile = os.path.join(openembeddeddir, bb.data.expand("${P}-${PR}.showdata.dump",d))
+	dumpfile = os.path.join(openembeddeddir, bb.data.expand("${P}-${PKG_PR}.showdata.dump",d))
 	
 	try:
 		os.mkdir(openembeddeddir)
@@ -97,8 +97,8 @@ sourcepkg_do_create_diff_gz(){
 		cp $i $src_tree/${DISTRO}/files
 	done
 	
-	oenote "Creating .diff.gz in ${DEPLOY_DIR_SRC}/${P}-${PR}.diff.gz"
-	LC_ALL=C TZ=UTC0 diff --exclude-from=temp/exclude-from-file -Naur $src_tree.orig $src_tree | gzip -c > ${DEPLOY_DIR_SRC}/${P}-${PR}.diff.gz
+	oenote "Creating .diff.gz in ${DEPLOY_DIR_SRC}/${P}-${PKG_PR}.diff.gz"
+	LC_ALL=C TZ=UTC0 diff --exclude-from=temp/exclude-from-file -Naur $src_tree.orig $src_tree | gzip -c > ${DEPLOY_DIR_SRC}/${P}-${PKG_PR}.diff.gz
 	rm -rf $src_tree.orig
 }
 
diff --git a/classes/tinderclient.bbclass b/classes/tinderclient.bbclass
index 0b7fc1d..1b28418 100644
--- a/classes/tinderclient.bbclass
+++ b/classes/tinderclient.bbclass
@@ -65,7 +65,7 @@ def tinder_format_http_post(d,status,log):
         "srcdate"      : data.getVar('SRCDATE', d, True),
         "PN"           : data.getVar('PN', d, True),
         "PV"           : data.getVar('PV', d, True),
-        "PR"           : data.getVar('PR', d, True),
+        "PR"           : data.getVar('PKG_PR', d, True),
         "FILE"         : data.getVar('FILE', d, True) or "N/A",
         "TARGETARCH"   : data.getVar('TARGET_ARCH', d, True),
         "TARGETFPU"    : data.getVar('TARGET_FPU', d, True) or "Unknown",
diff --git a/conf/bitbake.conf b/conf/bitbake.conf
index d3abb1e..50d2fb2 100644
--- a/conf/bitbake.conf
+++ b/conf/bitbake.conf
@@ -135,12 +135,12 @@ ASSUME_PROVIDED = "\
 ##################################################################
 PN = "${@bb.parse.BBHandler.vars_from_file(bb.data.getVar('FILE',d),d)[0] or 'defaultpkgname'}"
 PV = "${@bb.parse.BBHandler.vars_from_file(bb.data.getVar('FILE',d),d)[1] or '1.0'}"
-FILE_PR = "${@bb.parse.BBHandler.vars_from_file(bb.data.getVar('FILE',d),d)[2] or 'r0'}"
-PR = "${FILE_PR}${DISTRO_PR}"
-PF = "${PN}-${EXTENDPE}${PV}-${PR}"
+PR = "${@bb.parse.BBHandler.vars_from_file(bb.data.getVar('FILE',d),d)[2] or 'r0'}"
+PKG_PR = "${PR}${DISTRO_PR}"
+PF = "${PN}-${EXTENDPE}${PV}-${PKG_PR}"
 EXTENDPE = "${@['','${PE\x7d_'][bb.data.getVar('PE',d,1) > 0]}"
 EXTENDPEVER = "${@['','${PE\x7d:'][bb.data.getVar('PE',d,1) > 0]}"
-DEBPV = "${EXTENDPEVER}${PV}-${PR}"
+DEBPV = "${EXTENDPEVER}${PV}-${PKG_PR}"
 P = "${PN}-${PV}"
 
 ##################################################################
@@ -152,7 +152,7 @@ DISTRO_PR ?= ""
 
 SECTION = "base"
 PRIORITY = "optional"
-DESCRIPTION = "Version ${PV}-${PR} of package ${PN}"
+DESCRIPTION = "Version ${PV}-${PKG_PR} of package ${PN}"
 LICENSE = "unknown"
 MAINTAINER = "OpenEmbedded Team <openembedded-devel@lists.openembedded.org>"
 HOMEPAGE = "unknown"

Re: FILE_PR and my requirements Re: Some open issues

[Phil Blundell]

On Sat, 2008-10-18 at 15:37 +0200, Holger Freyther wrote:
> On Saturday 18 October 2008 15:20:05 Holger Freyther wrote:
> > On Saturday 18 October 2008 14:32:37 Holger Freyther wrote:
> >
> > More in depth:
> - We need to make sure that people do not introduce PR but use FILE_PR
> 
> Oops I missed my requirements:
> 	- Easy to understand what is going on
> 	- Package name and Workdir matching with each other
> 	- Full rebuild on DISTRO_PR bump
> 	- Not reusing the same build directory

These seem like fine requirements.  The additional one from me, which I
try to apply to more or less any change in OE, is that any change to the
grammar of .bb files should be both forwards and (to the greatest extent
possible) backwards compatible.  That is, unless there are overriding
reasons why it is impossible, two use cases should both be supported:

(a) backporting cherry-picked recipes from the .dev tree to the stable
branch; and

(b) merging recipes from other, parallel development trees that might be
using slightly older standards or processes.

> And here two patches. The amount of changes is a lot smaller. The downside is 
> it is not as easy to understand as changin PR to FILE_PR (think of the people 
> merging OE into their private trees that know little about the classes...). 
> Anyway this is something we can talk about..

Right, this seems like a good solution.  I'm not sure that it really is
any harder to understand than the FILE_PR thing, and to be honest the
internals of the packaging classes are already cryptic enough that I
doubt many people make any attempt to comprehend them in detail.  The
relationship between PR and PKG_PR is clearly shown in bitbake.conf,
rather than buried in some obscure piece of python, so there should be
no difficulty in understanding for anybody who looks at it.

Requiring a new bitbake version is obviously a bit of a shame but there
is plenty of precedent for that, and the bitbake change looks like it
should be backwards compatible with no problem.

I haven't actually tested the patches so I can't speak for their correct
functionality.  But the general approach seems like a good one to me.

p.

Re: FILE_PR and my requirements Re: Some open issues

[Richard Purdie]

On Sat, 2008-10-18 at 15:35 +0100, Phil Blundell wrote:
> On Sat, 2008-10-18 at 15:37 +0200, Holger Freyther wrote:
> > On Saturday 18 October 2008 15:20:05 Holger Freyther wrote:
> > > On Saturday 18 October 2008 14:32:37 Holger Freyther wrote:
> > >
> > > More in depth:
> > - We need to make sure that people do not introduce PR but use FILE_PR
> > 
> > Oops I missed my requirements:
> > 	- Easy to understand what is going on
> > 	- Package name and Workdir matching with each other
> > 	- Full rebuild on DISTRO_PR bump
> > 	- Not reusing the same build directory
> 
> These seem like fine requirements.  The additional one from me, which I
> try to apply to more or less any change in OE, is that any change to the
> grammar of .bb files should be both forwards and (to the greatest extent
> possible) backwards compatible.  That is, unless there are overriding
> reasons why it is impossible, two use cases should both be supported:
> 
> (a) backporting cherry-picked recipes from the .dev tree to the stable
> branch; and
> 
> (b) merging recipes from other, parallel development trees that might be
> using slightly older standards or processes.
> 
> > And here two patches. The amount of changes is a lot smaller. The downside is 
> > it is not as easy to understand as changin PR to FILE_PR (think of the people 
> > merging OE into their private trees that know little about the classes...). 
> > Anyway this is something we can talk about..
> 
> Right, this seems like a good solution.  I'm not sure that it really is
> any harder to understand than the FILE_PR thing, and to be honest the
> internals of the packaging classes are already cryptic enough that I
> doubt many people make any attempt to comprehend them in detail.  The
> relationship between PR and PKG_PR is clearly shown in bitbake.conf,
> rather than buried in some obscure piece of python, so there should be
> no difficulty in understanding for anybody who looks at it.

I talked with Holger on irc about this. The need is primarily to have a
way of regenerating a full set of packages after some package abi
change. I raised the idea of using DISTRO_PR in a somewhat similar way
to SANITY_ABI where if it changes the user is warned about it and then
can handle as they see fit. DISTRO_PR is appended to the PR values at
package generation time if set. For autobuilders this wouldn't be good
so for those cases I'm suggesting they incorporate DISTRO_PR into the
TMPDIR value their autobuilder uses.

Yes, this kind of change may break some scripts out there. That is
unfortunate but we should use this opportunity to improve the scripts to
use commands like "bitbake -e | grep ^DEPLOY_DIR_IMAGES" and similar to
find things rather than making assumptions about path layout.

The advantages to this approach are that we don't need changes to
bitbake, we don't break the file format, we don't change the file layout
and we leave options open for other requirements that may come along in
the future.

Holger started a patch implementing this which I've added some code
which is shared as:

http://git.openembedded.net/?p=openembedded.git;a=shortlog;h=refs/heads/shared/file-pr-revert

I also took the opportunity to split the ABI variables into a separate
clearly identifiable conf file like Poky.

Cheers,

Richard

Re: FILE_PR and my requirements Re: Some open issues

[Holger Freyther]

On Saturday 18 October 2008 19:18:57 Richard Purdie wrote:

> I talked with Holger on irc about this. The need is primarily to have a
> way of regenerating a full set of packages after some package abi
> change. I raised the idea of using DISTRO_PR in a somewhat similar way
> to SANITY_ABI where if it changes the user is warned about it and then
> can handle as they see fit. DISTRO_PR is appended to the PR values at
> package generation time if set. For autobuilders this wouldn't be good
> so for those cases I'm suggesting they incorporate DISTRO_PR into the
> TMPDIR value their autobuilder uses.

There are some things we agree on and some things that we need to find 
agreement on. But with the proposed changes we get to a point where we can 
discuss. My fast commit was clearly against the policies and I try to do 
better.


>
> Yes, this kind of change may break some scripts out there. That is
> unfortunate but we should use this opportunity to improve the scripts to
> use commands like "bitbake -e | grep ^DEPLOY_DIR_IMAGES" and similar to
> find things rather than making assumptions about path layout.

Yes, autobuilders and script writers are encouraged to use bitbake to get the 
paths instead of guessing where files are located.


z.

Re: FILE_PR and my requirements Re: Some open issues

[Holger Freyther]

On Saturday 18 October 2008 19:18:57 Richard Purdie wrote:

> http://git.openembedded.net/?p=openembedded.git;a=shortlog;h=refs/heads/sha
>red/file-pr-revert

When should we merge this?

z.

Re: FILE_PR and my requirements Re: Some open issues

[Richard Purdie]

On Tue, 2008-10-21 at 12:25 +0200, Holger Freyther wrote:
> On Saturday 18 October 2008 19:18:57 Richard Purdie wrote:
> 
> > http://git.openembedded.net/?p=openembedded.git;a=shortlog;h=refs/heads/sha
> >red/file-pr-revert
> 
> When should we merge this?

Now?

Richard	

Re: FILE_PR and my requirements Re: Some open issues

[Holger Freyther]

On Tuesday 21 October 2008 13:24:43 Richard Purdie wrote:
> On Tue, 2008-10-21 at 12:25 +0200, Holger Freyther wrote:
> > On Saturday 18 October 2008 19:18:57 Richard Purdie wrote:
> > > http://git.openembedded.net/?p=openembedded.git;a=shortlog;h=refs/heads
> > >/sha red/file-pr-revert
> >
> > When should we merge this?
>
> Now?


Me or You?

z.

Re: Some open issues

[Richard Purdie]

On Sat, 2008-10-18 at 14:32 +0200, Holger Freyther wrote:
> On Saturday 18 October 2008 11:03:29 Richard Purdie wrote:
> > What is most annoying is that given a bit longer I think we could have
> > done something that would have meant this was unnecessary, specifically
> > inserted the revision into the package at package_*.bbclass time where
> > we can manipulated PR as needed. This combined with a staging ABI change
> > would have been all that was needed. If staging ABI isn't enough, we can
> > insert the modified PR into STAMPS instead of the real PR or some other
> > magic. My point is that there are better options than FILE_PR, it just
> > needs some thought. The fact the testing branch had so many merge issues
> > should have meant a better idea was sought, not that is should be
> > committed ASAP.
> 
> I don't agree with this. This means package names will not match with the on 
> dir directory name. For me this was the strongest argument against doing it 
> automatically in the packaging bbclass. I really don't like the idea that if 
> I bump the DISTRO_PR the existing build directories will be recycled and 
> packages will be rebuild in there. That mostly defeats the purpose of a 
> deterministic tool. And I really don't like the idea of adding more python 
> black magic that is mangling PR... :)

We'll take this one to a separate thread.

> > 2. The Git conversion including the BKCVS data.
> >
> > I'd made it quite clear this should have been a tree graft and this
> > wasn't done so we're now stuck with broken history :(. This is pretty
> > frustrating since I'd repeatedly said not to include it and went to the
> > effort of gathering my conversion data and sending it to Jan who then
> > didn't realise what I meant by graft (though no fault of his own).
> 
> Should we start over from scratch and rebase what we have already added? Will 
> this be the _last_ time we consider it?

As far as I'm concerned, yes. This is a one off problem.

> > 4. Bitbake changes
> >
> > These should go to the bitbake list as well as the OE list and should be
> > discussed. I've raised issues with patches which have been ignored and
> > these patches have now just need committed. I'm not happy about the
> > process that was used :(. I know people have various commitments but we
> > need to try and stick to some kind of process for this kind of thing.
> 
> ???? Things have been ignored... three times...

I tried for a couple of *months* after the first versions of those
patches were published to talk to you on irc about them and you were
always too busy. I did reply on the mailing list about them but we never
got a discussion going about things. You at least *knew* I wanted too
discuss them.

To tell me I ignored them is just plain wrong.

> > Where from here?
> >
> > I'd actually like to a strong line on this and suggest we revert the
> > FILE_PR change since its badly thought out and also that we consider
> > redoing the git conversion ASAP and the replaying the recent commits
> > before its too late to get rid of the corruption in there. This is
> > probably going to have to go to a core team vote since its a pretty big
> > change to suggest but opinions are welcome.
> 
> Do whatever you want. 

This is the attitude that gets us into trouble. Its not what I want, its
a question of what is best for OE.

> I think FILE_PR is the right thing to do, feel free to 
> invest the time to redo the conversion, the scripts are public and Jan might 
> be able to upload the "source" for git-fast-import, please also coordinate 
> the git-rebase (we will lose merges but that seems okay, but I did merge the 
> FILE_PR changes on purpose for ease of reverting), make sure no one is 
> merging old history with new history (this was not a big problem with the 
> trial but will be one now)...

Right, if we do this, we need to do this carefully. If it needs to be me
that finds the time to do this, so be it, I'm the one complaining.
Obviously some tips on how to make the conversion would be good but I
will do it from scratch and blind if that pain makes you feel better.

Cheers,

Richard

Re: Some open issues

[Phil Blundell]

On Sat, 2008-10-18 at 10:03 +0100, Richard Purdie wrote:
> I'd actually like to a strong line on this and suggest
> [...] that we consider
> redoing the git conversion ASAP and the replaying the recent commits
> before its too late to get rid of the corruption in there. This is
> probably going to have to go to a core team vote since its a pretty big
> change to suggest but opinions are welcome.

Obviously I'm not a core team member, but seeing as you asked for
opinions... :-}

I do struggle to see why this is such a pressing issue right now.  As I
understand it, the "corruption" in question is basically mangled data
from the BK history.  This doesn't seem like it is obviously going to
get any worse over time, nor does it seem like the contagion is likely
to spread to other data, nor is the corruption going to be visible at
the tip of any recent checkout.  Also, we have now had several days of
activity on the live git tree, and we would presumably want to preserve
those changesets on any putative re-import.

Assuming the above to be the case, I don't fully understand either:

i) why this is a terribly big issue in the first place (i.e. what the
real-world impact of the corruption is going to be); or

ii) why, if we don't act now, it might be "too late" to solve the
problem in the future

Of course, it's entirely possible that my assumptions above are wrong.
I'd be interested to know the real situation, anyway.

p.

Re: Some open issues

[Richard Purdie]

On Sat, 2008-10-18 at 15:59 +0100, Phil Blundell wrote:
> I do struggle to see why this is such a pressing issue right now.  As I
> understand it, the "corruption" in question is basically mangled data
> from the BK history.  This doesn't seem like it is obviously going to
> get any worse over time, nor does it seem like the contagion is likely
> to spread to other data, nor is the corruption going to be visible at
> the tip of any recent checkout.  Also, we have now had several days of
> activity on the live git tree, and we would presumably want to preserve
> those changesets on any putative re-import.
> 
> Assuming the above to be the case, I don't fully understand either:
> 
> i) why this is a terribly big issue in the first place (i.e. what the
> real-world impact of the corruption is going to be); or

There is no impact on the current metadata, just the history. It depends
how much value we place on that.

At present we can't at some future date add in a good version of the
BKCVS data as easily as we could if it had been done by a tree graft
originally. The data there at the moment is pretty useless for actually
finding history (I know since I've tried to use it).

> ii) why, if we don't act now, it might be "too late" to solve the
> problem in the future

As you point out, we have a certain number of commits already. At the
moment we can probably deal with these but the longer we leave it, the
more commits and the more I'd not want to rebase things. We also have
the FILE_PR issue to consider and we need to revert that commit by some
means and find a better solution (I think even zecke agrees with that
but we'll discuss that in another thread).

Cheers,

Richard

Re: Some open issues

[Tom Rini]

On Sat, Oct 18, 2008 at 04:37:20PM +0100, Richard Purdie wrote:
> On Sat, 2008-10-18 at 15:59 +0100, Phil Blundell wrote:
[snip]
> > i) why this is a terribly big issue in the first place (i.e. what the
> > real-world impact of the corruption is going to be); or
> 
> There is no impact on the current metadata, just the history. It depends
> how much value we place on that.

And with 'git bisect' good history is very valuable for fixing
long-standing bugs.  Or at least can be.

-- 
Tom Rini

Re: Some open issues

[Richard Purdie]

On Sat, 2008-10-18 at 16:37 +0100, Richard Purdie wrote:
> On Sat, 2008-10-18 at 15:59 +0100, Phil Blundell wrote:
> > i) why this is a terribly big issue in the first place (i.e. what the
> > real-world impact of the corruption is going to be); or
> 
> There is no impact on the current metadata, just the history. It depends
> how much value we place on that.
> 
> At present we can't at some future date add in a good version of the
> BKCVS data as easily as we could if it had been done by a tree graft
> originally. The data there at the moment is pretty useless for actually
> finding history (I know since I've tried to use it).
> 
> > ii) why, if we don't act now, it might be "too late" to solve the
> > problem in the future
> 
> As you point out, we have a certain number of commits already. At the
> moment we can probably deal with these but the longer we leave it, the
> more commits and the more I'd not want to rebase things. We also have
> the FILE_PR issue to consider and we need to revert that commit by some
> means and find a better solution (I think even zecke agrees with that
> but we'll discuss that in another thread).

I've run some tests and to fix things, someone needs to make a checkout
and then run:

git filter-branch --parent-filter  'test $GIT_COMMIT = e3d1546d2c1ec9fd00af7780ba41250496153b15 && echo || cat' -- --all

which will remove the bitkeeper data from the history. We can then graft
it back in with:

echo "b97239969db33bf4be1b5f5807eae4d1d2987ca1 c8e5702127e507e82e6f68a4b8c546803accea9d fc6dcc1f1f0eabd8f2a465f219ffcd2554deec04" > .git/info/grafts

(where b97239969db33bf4be1b5f5807eae4d1d2987ca1 is the revision
e3d1546d2c1ec9fd00af7780ba41250496153b15 became. Just to illustrate how
broken the bkcvs conversion is, look at the diff between the git and
bkcvs history:

git diff fc6dcc1f1f0eabd8f2a465f219ffcd2554deec04  b97239969db33bf4be1b5f5807eae4d1d2987ca1

The reason this is huge is that we seem to have ton of zero length
files. Given its so totally broken I'm keen to do the above so we need a
plan. I propose:

a) repo goes read only
b) checkout is made
c) conversion is run
d) sanity checks are made
e) new revisions are pushed
f) graft is added to the main OE repo
g) commits based on the old history are blocked
h) repo is made writable again
i) people are advised to rebase any unpushed changes

Is anyone strongly against this? If not, when should we do this?

Richard

Re: Some open issues

[Michael 'Mickey' Lauer]

Am Sunday 19 October 2008 20:06:50 schrieb Richard Purdie:
> The reason this is huge is that we seem to have ton of zero length
> files. Given its so totally broken I'm keen to do the above so we need a
> plan. I propose:
>
> a) repo goes read only
> b) checkout is made
> c) conversion is run
> d) sanity checks are made
> e) new revisions are pushed
> f) graft is added to the main OE repo
> g) commits based on the old history are blocked
> h) repo is made writable again
> i) people are advised to rebase any unpushed changes
>
> Is anyone strongly against this? If not, when should we do this?

No objection. Please synchronize with Jan Lübbe, he has lots of experience 
with the conversion. Feel free to chose the time/date when you (both) have 
resources to do that. Please announce a day before though.

Thanks,

Mickey.

Re: Some open issues

[Koen Kooi]

On 18-10-2008 11:03, Richard Purdie wrote:
> Hi Guys,
>
> I have a few issues with the way certain things have happened recently.
>
> 1. The FILE_PR change.
>
> This was mentioned in an email on Wednesday with the title "[oe] [RFC]
> Enable --hash-style=both for all recent gcc4 targets" at 9am. At 8pm we
> have "I decided to land now as PRs are changing all the time and keeping
> up with things is pretty hard...". This is not in keeping with the major
> changes policy we agreed by any stretch of the imagination.
>
> This change breaks compatibility with everyone's overlays and creates
> the nightmare scenario of external OE "branches" being forced into the
> change or forever being unable to sync (Openmoko and Poky spring to
> mind).
>
> What is most annoying is that given a bit longer I think we could have
> done something that would have meant this was unnecessary, specifically
> inserted the revision into the package at package_*.bbclass time where
> we can manipulated PR as needed. This combined with a staging ABI change
> would have been all that was needed. If staging ABI isn't enough, we can
> insert the modified PR into STAMPS instead of the real PR or some other
> magic. My point is that there are better options than FILE_PR, it just
> needs some thought. The fact the testing branch had so many merge issues
> should have meant a better idea was sought, not that is should be
> committed ASAP.
>
> 2. The Git conversion including the BKCVS data.
>
> I'd made it quite clear this should have been a tree graft and this
> wasn't done so we're now stuck with broken history :(. This is pretty
> frustrating since I'd repeatedly said not to include it and went to the
> effort of gathering my conversion data and sending it to Jan who then
> didn't realise what I meant by graft (though no fault of his own).

I have no strong opinion on the BK stuff, but it would be nice to have 
the history in the same repo.

> 3. Merging Bitbake into OE
>
> People are saying things like "Might be a chance to reconsider
> maintaining BitBake in the OE repository.". Could people please talk to
> the bitbake maintainer about things like this *before* encouraging it in
> public. If we need a release lets make one (which seems to be the real
> problem).

I do have a strong opinion on that :) Bitbake should be kept out of the 
.dev branch. I can see how things like the new stable branch *might* 
include a copy, but let's cross that bridge when we get there.

> 4. Bitbake changes
>
> These should go to the bitbake list as well as the OE list and should be
> discussed. I've raised issues with patches which have been ignored and
> these patches have now just need committed. I'm not happy about the
> process that was used :(. I know people have various commitments but we
> need to try and stick to some kind of process for this kind of thing.

I know very little about bitbake and don't contribute to it, so no 
opinion there.

> Where from here?
>
> I'd actually like to a strong line on this and suggest we revert the
> FILE_PR change since its badly thought out and also that we consider
> redoing the git conversion ASAP and the replaying the recent commits
> before its too late to get rid of the corruption in there. This is
> probably going to have to go to a core team vote since its a pretty big
> change to suggest but opinions are welcome.

<distro hat> The most important thing is the in the end distros can have 
a way to increase a global PR</distro hat>

<oe hat>I agree with your proposal</oe hat>

regards,

Koen

Re: Some open issues

[Rod Whitby]

I'm not on the core team, but wanted to say that I would support the decision of the core team if they agree that Richard's proposals are worth the effort to keep the architecture and history clean.
-- Rod

-----Original Message-----
From: Richard Purdie <rpurdie@rpsys.net>
Date: Saturday, Oct 18, 2008 7:37 pm
Subject: [oe] Some open issues
To: openembedded-devel <openembedded-devel@openembedded.org>Reply-To: openembedded-devel@lists.openembedded.org

Hi Guys,
>
>I have a few issues with the way certain things have happened recently.
>
>1. The FILE_PR change.
>
>This was mentioned in an email on Wednesday with the title "[oe] [RFC] Enable --hash-style=both for all recent gcc4 targets" at 9am. At 8pm we have "I decided to land now as PRs are changing all the time and keeping up with things is pretty hard...". This is not in keeping with the major changes policy we agreed by any stretch of the imagination.
>
>This change breaks compatibility with everyone's overlays and creates the nightmare scenario of external OE "branches" being forced into the change or forever being unable to sync (Openmoko and Poky spring to
>mind).
>
>What is most annoying is that given a bit longer I think we could have done something that would have meant this was unnecessary, specifically inserted the revision into the package at package_*.bbclass time where we can manipulated PR as needed. This combined with a staging ABI change would have been all that was needed. If staging ABI isn't enough, we can insert the modified PR into STAMPS instead of the real PR or some other magic. My point is that there are better options than FILE_PR, it just needs some thought. The fact the testing branch had so many merge issues should have meant a better idea was sought, not that is should be
>committed ASAP.
>
>2. The Git conversion including the BKCVS data.
>
>I'd made it quite clear this should have been a tree graft and this wasn't done so we're now stuck with broken history :(. This is pretty frustrating since I'd repeatedly said not to include it and went to the effort of gathering my conversion data and sending it to Jan who then didn't realise what I meant by graft (though no fault of his own).
>
>3. Merging Bitbake into OE
>
>People are saying things like "Might be a chance to reconsider
>maintaining BitBake in the OE repository.". Could people please talk to the bitbake maintainer about things like this *before* encouraging it in public. If we need a release lets make one (which seems to be the real problem).
>
>4. Bitbake changes
>
>These should go to the bitbake list as well as the OE list and should be discussed. I've raised issues with patches which have been ignored and these patches have now just need committed. I'm not happy about the
>process that was used :(. I know people have various commitments but we need to try and stick to some kind of process for this kind of thing.
>
>
>Where from here?
>
>I'd actually like to a strong line on this and suggest we revert the FILE_PR change since its badly thought out and also that we consider
>redoing the git conversion ASAP and the replaying the recent commits
>before its too late to get rid of the corruption in there. This is
>probably going to have to go to a core team vote since its a pretty big change to suggest but opinions are welcome.
>
>Richard
>
>
>
>_______________________________________________
>Openembedded-devel mailing list
>Openembedded-devel@lists.openembedded.org
>http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
>
>