[pacman-dev] [PATCH 0/5] RFC: epoch implementation

Dan McGee

8 Oct 2010 8 Oct '10

5:02 p.m.

Hey guys, This is a quick set of patches to implement the use of epoch in our packages, and remove the 'force' option. It adds a lot more flexibility and should prevent weird upgrade/downgrade interaction between different repositories, and also removes the need to munge versions as often. The pacman stuff is pretty well tested via the current suite of pactests, although we will want to add some new tests specifically playing with the epoch value in the test packages. Anyway, thoughts/comments/suggestions welcome. The changes are not too invasive and the only real complexity comes from trying to remain backward and forward compatible. This addresses bugs such as FS#14887, FS#19153, FS#19639 along with many a conversation we've had in the past on this list. -Dan Dan McGee (5): Add epoch support to pacman/libalpm Update documentation to reflect new epoch package variable Make repo-add and makepkg epoch-aware Add epoch support to pactest Update contrib/ for epoch contrib/PKGBUILD.vim | 10 ++++++++-- contrib/bacman | 3 +++ doc/PKGBUILD.5.txt | 22 ++++++++++++---------- doc/pacman.8.txt | 3 +++ lib/libalpm/alpm.h | 2 +- lib/libalpm/be_files.c | 16 +++++++++++++--- lib/libalpm/be_package.c | 2 ++ lib/libalpm/package.c | 24 ++++++++++++++---------- lib/libalpm/package.h | 2 +- scripts/makepkg.sh.in | 6 ++---- scripts/repo-add.sh.in | 7 ++++++- test/pacman/pmdb.py | 18 +++++++----------- test/pacman/pmpkg.py | 5 +++-- test/pacman/util.py | 4 ++++ 14 files changed, 79 insertions(+), 45 deletions(-) -- 1.7.3.1

Show replies by date

Dan McGee

8 Oct 8 Oct

5:02 p.m.

New subject: [pacman-dev] [PATCH 1/5] Add epoch support to pacman/libalpm

This will allow for better control of what was previously the 'force' option in a PKGBUILD and transferred into the built package. Signed-off-by: Dan McGee --- lib/libalpm/alpm.h | 2 +- lib/libalpm/be_files.c | 16 +++++++++++++--- lib/libalpm/be_package.c | 2 ++ lib/libalpm/package.c | 24 ++++++++++++++---------- lib/libalpm/package.h | 2 +- 5 files changed, 31 insertions(+), 15 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 0c01f21..3dea6be 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -235,7 +235,7 @@ size_t alpm_pkg_changelog_read(void *ptr, size_t size, /*int alpm_pkg_changelog_feof(const pmpkg_t *pkg, void *fp);*/ int alpm_pkg_changelog_close(const pmpkg_t *pkg, void *fp); int alpm_pkg_has_scriptlet(pmpkg_t *pkg); -int alpm_pkg_has_force(pmpkg_t *pkg); +int alpm_pkg_get_epoch(pmpkg_t *pkg); off_t alpm_pkg_download_size(pmpkg_t *newpkg); alpm_list_t *alpm_pkg_unused_deltas(pmpkg_t *pkg); diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index 0f055e0..7fc6051 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -626,8 +626,17 @@ int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) STRDUP(linedup, _alpm_strtrim(line), goto error); info->replaces = alpm_list_add(info->replaces, linedup); } + } else if(strcmp(line, "%EPOCH%") == 0) { + if(fgets(line, sizeof(line), fp) == NULL) { + goto error; + } + info->epoch = atoi(_alpm_strtrim(line)); } else if(strcmp(line, "%FORCE%") == 0) { - info->force = 1; + /* For backward compatibility, treat force as a really big epoch + * but only if we didn't already have a known epoch value. */ + if(!info->epoch) { + info->epoch = INT_MAX; + } } } fclose(fp); @@ -826,8 +835,9 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } fprintf(fp, "\n"); } - if(info->force) { - fprintf(fp, "%%FORCE%%\n\n"); + if(info->epoch) { + fprintf(fp, "%%EPOCH%%\n" + "%d\n\n", info->epoch); } if(local) { if(info->url) { diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 3d8c4e3..2291a48 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -75,6 +75,8 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg) STRDUP(newpkg->version, ptr, RET_ERR(PM_ERR_MEMORY, -1)); } else if(strcmp(key, "pkgdesc") == 0) { STRDUP(newpkg->desc, ptr, RET_ERR(PM_ERR_MEMORY, -1)); + } else if(strcmp(key, "epoch") == 0) { + newpkg->epoch = atoi(ptr); } else if(strcmp(key, "group") == 0) { newpkg->groups = alpm_list_add(newpkg->groups, strdup(ptr)); } else if(strcmp(key, "url") == 0) { diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 717d32c..30ea3a9 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -295,7 +295,7 @@ alpm_list_t SYMEXPORT *alpm_pkg_get_groups(pmpkg_t *pkg) return pkg->groups; } -int SYMEXPORT alpm_pkg_has_force(pmpkg_t *pkg) +int SYMEXPORT alpm_pkg_get_epoch(pmpkg_t *pkg) { ALPM_LOG_FUNC; @@ -306,7 +306,7 @@ int SYMEXPORT alpm_pkg_has_force(pmpkg_t *pkg) if(pkg->origin == PKG_FROM_CACHE && !(pkg->infolevel & INFRQ_DESC)) { _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC); } - return pkg->force; + return pkg->epoch; } alpm_list_t SYMEXPORT *alpm_pkg_get_depends(pmpkg_t *pkg) @@ -649,7 +649,7 @@ pmpkg_t *_alpm_pkg_dup(pmpkg_t *pkg) newpkg->size = pkg->size; newpkg->isize = pkg->isize; newpkg->scriptlet = pkg->scriptlet; - newpkg->force = pkg->force; + newpkg->epoch = pkg->epoch; newpkg->reason = pkg->reason; newpkg->licenses = alpm_list_strdup(pkg->licenses); @@ -736,21 +736,25 @@ void _alpm_pkg_free_trans(pmpkg_t *pkg) pkg->removes = NULL; } -/* Is spkg an upgrade for locapkg? */ +/* Is spkg an upgrade for localpkg? */ int _alpm_pkg_compare_versions(pmpkg_t *spkg, pmpkg_t *localpkg) { - int cmp = 0; + int spkg_epoch, localpkg_epoch; ALPM_LOG_FUNC; - cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg), - alpm_pkg_get_version(localpkg)); + spkg_epoch = alpm_pkg_get_epoch(spkg); + localpkg_epoch = alpm_pkg_get_epoch(localpkg); - if(cmp < 0 && alpm_pkg_has_force(spkg)) { - cmp = 1; + if(spkg_epoch > localpkg_epoch) { + return(1); + } else if(spkg_epoch < localpkg_epoch) { + return(-1); } - return(cmp); + /* equal epoch values, move on to version comparison */ + return alpm_pkg_vercmp(alpm_pkg_get_version(spkg), + alpm_pkg_get_version(localpkg)); } /* Helper function for comparing packages diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index 14f81f9..8273760 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -50,7 +50,7 @@ struct __pmpkg_t { off_t isize; off_t download_size; int scriptlet; - int force; + int epoch; pmpkgreason_t reason; alpm_list_t *licenses; alpm_list_t *replaces; -- 1.7.3.1

Allan McRae

9 Oct 9 Oct

5:31 a.m.

New subject: [pacman-dev] [PATCH 1/5] Add epoch support to pacman/libalpm

On 09/10/10 01:02, Dan McGee wrote: <snip>

...

-/* Is spkg an upgrade for locapkg? */ +/* Is spkg an upgrade for localpkg? */ int _alpm_pkg_compare_versions(pmpkg_t *spkg, pmpkg_t *localpkg) { - int cmp = 0; + int spkg_epoch, localpkg_epoch;

ALPM_LOG_FUNC;

- cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg), - alpm_pkg_get_version(localpkg)); + spkg_epoch = alpm_pkg_get_epoch(spkg); + localpkg_epoch = alpm_pkg_get_epoch(localpkg);

- if(cmp< 0&& alpm_pkg_has_force(spkg)) { - cmp = 1; + if(spkg_epoch> localpkg_epoch) { + return(1); + } else if(spkg_epoch< localpkg_epoch) { + return(-1); }

- return(cmp); + /* equal epoch values, move on to version comparison */ + return alpm_pkg_vercmp(alpm_pkg_get_version(spkg), + alpm_pkg_get_version(localpkg)); }

How about: /* Is spkg an upgrade for localpkg? */ int _alpm_pkg_compare_versions(pmpkg_t *spkg, pmpkg_t *localpkg) { int cmp, spkg_epoch, localpkg_epoch; ALPM_LOG_FUNC; cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg), alpm_pkg_get_version(localpkg)); spkg_epoch = alpm_pkg_get_epoch(spkg); localpkg_epoch = alpm_pkg_get_epoch(localpkg); if(spkg_epoch > localpkg_epoch && cmp != 0) { return(1); } else if(spkg_epoch < localpkg_epoch) { return(-1); } return(cmp); } So, when 'force' option is in effect, spkg_epoch = INT_MAX, localpkg_epoch = 0, but the versions are the same so we ignore it. Allan

Dan McGee

6:10 a.m.

New subject: [pacman-dev] [PATCH 1/5] Add epoch support to pacman/libalpm

On Fri, Oct 8, 2010 at 10:31 PM, Allan McRae wrote:

...

On 09/10/10 01:02, Dan McGee wrote: <snip>

...
-/* Is spkg an upgrade for locapkg? */ +/* Is spkg an upgrade for localpkg? */ int _alpm_pkg_compare_versions(pmpkg_t *spkg, pmpkg_t *localpkg) { - int cmp = 0; + int spkg_epoch, localpkg_epoch;

ALPM_LOG_FUNC;

- cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg), - alpm_pkg_get_version(localpkg)); + spkg_epoch = alpm_pkg_get_epoch(spkg); + localpkg_epoch = alpm_pkg_get_epoch(localpkg);

- if(cmp< 0&& alpm_pkg_has_force(spkg)) { - cmp = 1; + if(spkg_epoch> localpkg_epoch) { + return(1); + } else if(spkg_epoch< localpkg_epoch) { + return(-1); }

- return(cmp); + /* equal epoch values, move on to version comparison */ + return alpm_pkg_vercmp(alpm_pkg_get_version(spkg), + alpm_pkg_get_version(localpkg)); }

How about:

/* Is spkg an upgrade for localpkg? */ int _alpm_pkg_compare_versions(pmpkg_t *spkg, pmpkg_t *localpkg) { int cmp, spkg_epoch, localpkg_epoch;

ALPM_LOG_FUNC;

cmp = alpm_pkg_vercmp(alpm_pkg_get_version(spkg), alpm_pkg_get_version(localpkg));

spkg_epoch = alpm_pkg_get_epoch(spkg); localpkg_epoch = alpm_pkg_get_epoch(localpkg);

if(spkg_epoch > localpkg_epoch && cmp != 0) { return(1); } else if(spkg_epoch < localpkg_epoch) { return(-1); }

return(cmp); }

So, when 'force' option is in effect, spkg_epoch = INT_MAX, localpkg_epoch = 0, but the versions are the same so we ignore it.

Works great now, but what if there is a legit reason to deem this as an upgrade? Throw out the 'force' and look at it as epochs 1 and 2 or something, so you don't get caught up just thinking this is an old problem. Epoch *always* has to win or it doesn't work correctly. Do we not record the force option in the local database right now? That is quite unfortunate. I think we need to make an exception for the force case, and in the future, we will always have epoch recorded in both sync and local DBs. Counterexample to your code above: Project releases version 1.0, 1.1, and 2.0. Project decides it wasn't ready for 2.0, so abandons that branch and goes back to 1.3. We need to use epoch here because we were quick to package it. Later, because someone wasn't thinking clearly, they release 2.0 again. Contrived, sure, but it would break your code (if the person still had 2.0 installed they would never see the new 2.0). -Dan

Dan McGee

8 Oct 8 Oct

5:02 p.m.

New subject: [pacman-dev] [PATCH 2/5] Update documentation to reflect new epoch package variable

Signed-off-by: Dan McGee --- doc/PKGBUILD.5.txt | 22 ++++++++++++---------- doc/pacman.8.txt | 3 +++ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 684800e..c85220b 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -50,7 +50,7 @@ similar to `$_basekernver`. *pkgrel*:: This is the release number specific to the Arch Linux release. This allows package maintainers to make updates to the package's configure - flags, for example. A pkgrel of 1 is typically used for each upstream + flags, for example. A pkgrel of '1' is typically used for each upstream software release and is incremented for intermediate PKGBUILD updates. The variable is not allowed to contain hyphens. @@ -58,6 +58,15 @@ similar to `$_basekernver`. This should be a brief description of the package and its functionality. Try to keep the description to one line of text. +*epoch*:: + Used to force the package to be seen as newer than any previous versions + with a lower epoch, even if the version number would normally not trigger + such an upgrade. This value is required to be a positive integer; the + default value if left unspecified is '0'. This is useful when the version + numbering scheme of a package changes (or is alphanumeric), breaking normal + version comparison logic. See linkman:pacman[8] for more information on + version comparisons. + *url*:: This field contains a URL that is associated with the software being packaged. This is typically the project's website. @@ -186,8 +195,8 @@ name. The syntax is: `source=('filename::url')`. in the options array. To reverse the default behavior, place an ``!'' at the front of the option. Only specify the options you specifically want to override, the rest will be taken from linkman:makepkg.conf[5]. - *NOTE:* 'force' is a special option only used in a linkman:PKGBUILD[5], - do not use it unless you know what you are doing. + *NOTE:* 'force' is a now-removed option in favor of the top level 'epoch' + variable. *strip*;; Strip symbols from binaries and libraries. If you frequently @@ -224,13 +233,6 @@ name. The syntax is: `source=('filename::url')`. `!makeflags` with select packages that have problems building with custom makeflags such as `-j2` (or higher). - *force*;; - Force the package to be upgraded by a pacman system upgrade - operation, even if the version number would normally not trigger - such an upgrade. This is useful when the version numbering scheme - of a package changes (or is alphanumeric). See linkman:pacman[8] for - more information on version comparisons. - build() Function ---------------- diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index de1f51f..83475b3 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -79,6 +79,9 @@ to determine which packages need upgrading. This behavior operates as follows: 1.0a < 1.0alpha < 1.0b < 1.0beta < 1.0p < 1.0pre < 1.0rc < 1.0 Numeric: 1 < 1.0 < 1.1 < 1.1.1 < 1.2 < 2.0 < 3.0.0 ++ +Additionally, packages can have an 'epoch' value defined that will override any +version comparison and force an upgrade. *-T, \--deptest*:: Check dependencies; this is useful in scripts such as makepkg to check -- 1.7.3.1

Dan McGee

5:02 p.m.

New subject: [pacman-dev] [PATCH 3/5] Make repo-add and makepkg epoch-aware

Allow it to be a variable in the PKGBUILD as well as propagating it through to the built package and the package database. We leave some backward compatibility in place by placing the '%FORCE%' option in the database if the package contains an epoch; this will be used by older versions of pacman and more or less ignored by versions that use epoch. Signed-off-by: Dan McGee --- scripts/makepkg.sh.in | 6 ++---- scripts/repo-add.sh.in | 7 ++++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3a7a4d1..c4b2dfc 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -45,7 +45,7 @@ srcdir="$startdir/src" pkgdir="$startdir/pkg" packaging_options=('strip' 'docs' 'libtool' 'emptydirs' 'zipman' 'purge') -other_options=('ccache' 'distcc' 'makeflags' 'force') +other_options=('ccache' 'distcc' 'makeflags') splitpkg_overrides=('pkgver' 'pkgrel' 'pkgdesc' 'arch' 'license' 'groups' \ 'depends' 'optdepends' 'provides' 'conflicts' 'replaces' \ 'backup' 'options' 'install' 'changelog') @@ -923,14 +923,12 @@ write_pkginfo() { (( SPLITPKG )) && echo pkgbase = $pkgbase echo "pkgver = $pkgver-$pkgrel" echo "pkgdesc = $pkgdesc" + [[ $epoch ]] && echo "epoch = $epoch" echo "url = $url" echo "builddate = $builddate" echo "packager = $packager" echo "size = $size" echo "arch = $PKGARCH" - if [[ $(check_option force) = "y" ]]; then - echo "force = true" - fi [[ $license ]] && printf "license = %s\n" "${license[@]}" [[ $replaces ]] && printf "replaces = %s\n" "${replaces[@]}" diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 8ef940d..626de23 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -185,7 +185,7 @@ db_write_entry() { # blank out all variables local pkgfile="$1" - local pkgname pkgver pkgdesc csize size md5sum url arch builddate packager force \ + local pkgname pkgver pkgdesc epoch csize size md5sum url arch builddate packager force \ _groups _licenses _replaces _depends _conflicts _provides _optdepends local OLDIFS="$IFS" @@ -262,6 +262,11 @@ db_write_entry() [[ -n $builddate ]] && echo -e "%BUILDDATE%\n$builddate\n" >>desc [[ -n $packager ]] && echo -e "%PACKAGER%\n$packager\n" >>desc write_list_entry "REPLACES" "$_replaces" "desc" + # remain backward-compatible for now; put a force entry in the database + if [[ -n $epoch ]]; then + echo -e "%EPOCH%\n#epoch\n" >>desc + echo -e "%FORCE%\n" >>desc + fi [[ -n $force ]] && echo -e "%FORCE%\n" >>desc # create depends entry -- 1.7.3.1

Dan McGee

5:02 p.m.

New subject: [pacman-dev] [PATCH 4/5] Add epoch support to pactest

This adds epoch support to pactest, while still producing packages and database entries the same way makepkg and repo-add currently do in a backward compatible fashion (still including the 'force' option). Signed-off-by: Dan McGee --- test/pacman/pmdb.py | 18 +++++++----------- test/pacman/pmpkg.py | 5 +++-- test/pacman/util.py | 4 ++++ 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py index 41bd738..008ec0d 100755 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -154,6 +154,8 @@ def db_read(self, name): pkg.md5sum = fd.readline().strip("\n") elif line == "%REPLACES%": pkg.replaces = _getsection(fd) + elif line == "%EPOCH%": + pkg.epoch = int(fd.readline().strip("\n")) elif line == "%FORCE%": fd.readline() pkg.force = 1 @@ -202,12 +204,6 @@ def db_read(self, name): pkg.conflicts = _getsection(fd) elif line == "%PROVIDES%": pkg.provides = _getsection(fd) - # TODO this was going to be changed, but isn't anymore - #elif line == "%REPLACES%": - # pkg.replaces = _getsection(fd) - #elif line == "%FORCE%": - # fd.readline() - # pkg.force = 1 fd.close() pkg.checksum["depends"] = getmd5sum(filename) pkg.mtime["depends"] = getmtime(filename) @@ -266,6 +262,11 @@ def db_write(self, pkg): data.append(_mksection("FILENAME", pkg.filename())) if pkg.replaces: data.append(_mksection("REPLACES", pkg.replaces)) + if pkg.epoch: + data.append(_mksection("EPOCH", pkg.epoch)) + # for backward compatibility + if not pkg.force: + data.append(_mksection("FORCE", "")) if pkg.force: data.append(_mksection("FORCE", "")) if pkg.csize: @@ -307,11 +308,6 @@ def db_write(self, pkg): data.append(_mksection("CONFLICTS", pkg.conflicts)) if pkg.provides: data.append(_mksection("PROVIDES", pkg.provides)) - #if self.treename != "local": - # if pkg.replaces: - # data.append(_mksection("REPLACES", pkg.replaces)) - # if pkg.force: - # data.append(_mksection("FORCE", "")) if data: data.append("") filename = os.path.join(path, "depends") diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py index 1d55175..aaee28b 100755 --- a/test/pacman/pmpkg.py +++ b/test/pacman/pmpkg.py @@ -48,8 +48,9 @@ def __init__(self, name, version = "1.0-1"): self.csize = 0 self.reason = 0 self.md5sum = "" # sync only - self.replaces = [] # sync only (will be moved to depends) - self.force = 0 # sync only (will be moved to depends) + self.replaces = [] + self.force = 0 + self.epoch = 0 # depends self.depends = [] self.optdepends = [] diff --git a/test/pacman/util.py b/test/pacman/util.py index 657230e..2e9ffb9 100755 --- a/test/pacman/util.py +++ b/test/pacman/util.py @@ -153,6 +153,10 @@ def mkdescfile(filename, pkg): data.append("provides = %s" % i) for i in pkg.backup: data.append("backup = %s" % i) + if pkg.epoch: + data.append("epoch = %d" % pkg.epoch) + if not pkg.force: + data.append("force = 1") if pkg.force: data.append("force = 1") -- 1.7.3.1

Dan McGee

5:02 p.m.

New subject: [pacman-dev] [PATCH 5/5] Update contrib/ for epoch

Signed-off-by: Dan McGee --- contrib/PKGBUILD.vim | 10 ++++++++-- contrib/bacman | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/contrib/PKGBUILD.vim b/contrib/PKGBUILD.vim index 8b40ed7..c2b6045 100644 --- a/contrib/PKGBUILD.vim +++ b/contrib/PKGBUILD.vim @@ -40,7 +40,7 @@ syn match pbPkgverGroup /^pkgver=.*/ contains=pbIllegalPkgver,pbValidPkgver,pb_k " pkgrel syn keyword pb_k_pkgrel pkgrel contained -syn match pbValidPkgrel /[[:digit:]]*/ contained contains=pbIllegalPkgver +syn match pbValidPkgrel /[[:digit:]]*/ contained contains=pbIllegalPkgrel syn match pbIllegalPkgrel /[^[:digit:]=]\|=.*=/ contained syn match pbPkgrelGroup /^pkgrel=.*/ contains=pbIllegalPkgrel,pbValidPkgrel,pb_k_pkgrel,shDoubleQuote,shSingleQuote @@ -52,6 +52,12 @@ syn match pbValidPkgdesc /[^='"]\.\{,80}/ contained contains=pbIllegalPkgdesc syn match pbPkgdescGroup /^pkgdesc=.*/ contains=pbIllegalPkgdesc,pb_k_desc,pbValidPkgdesc,shDoubleQuote,shSingleQuote syn match pbPkgdescSign /[='"]/ contained +" epoch +syn keyword pb_k_epoch epoch contained +syn match pbValidEpoch /[[:digit:]]*/ contained contains=pbIllegalEpoch +syn match pbIllegalEpoch /[^[:digit:]=]\|=.*=/ contained +syn match pbEpochGroup /^epoch=.*/ contains=pbIllegalEpoch,pbValidEpoch,pb_k_epoch,shDoubleQuote,shSingleQuote + " url syn keyword pb_k_url url contained syn match pbValidUrl /['"]*$https\|http\|ftp$\:\/.*\.\+.*/ contained @@ -163,7 +169,7 @@ hi def link pbValidSha1sums Number " options syn keyword pb_k_options options contained -syn match pbOptions /$no$\?$strip\|docs\|libtool\|emptydirs\|zipman\|ccache\|distcc\|makeflags\|force$/ contained +syn match pbOptions /$no$\?$strip\|docs\|libtool\|emptydirs\|zipman\|ccache\|distcc\|makeflags$/ contained syn match pbOptionsNeg /\!/ contained syn match pbOptionsDeprec /no/ contained syn region pbOptionsGroup start=/^options=(/ end=/)/ contains=pb_k_options,pbOptions,pbOptionsNeg,pbOptionsDeprec,pbIllegalOption,shDoubleQuote,shSingleQuote diff --git a/contrib/bacman b/contrib/bacman index 6dd7839..18f3c5b 100755 --- a/contrib/bacman +++ b/contrib/bacman @@ -220,6 +220,9 @@ while read i; do %REPLACES%) echo "replaces = $i" >> .PKGINFO ;; + %EPOCH%) + echo "epoch = $i" >> .PKGINFO + ;; %FORCE%) echo "force = true" >> .PKGINFO ;; -- 1.7.3.1

Xavier Chantry

6:22 p.m.

On Fri, Oct 8, 2010 at 5:02 PM, Dan McGee wrote:

...

Hey guys,

This is a quick set of patches to implement the use of epoch in our packages, and remove the 'force' option. It adds a lot more flexibility and should prevent weird upgrade/downgrade interaction between different repositories, and also removes the need to munge versions as often.

The pacman stuff is pretty well tested via the current suite of pactests, although we will want to add some new tests specifically playing with the epoch value in the test packages.

Anyway, thoughts/comments/suggestions welcome. The changes are not too invasive and the only real complexity comes from trying to remain backward and forward compatible.

This addresses bugs such as FS#14887, FS#19153, FS#19639 along with many a conversation we've had in the past on this list.

I don't remember seeing any of these bugs, but I do remember complaining several times about force and advocating use of epoch, so that's very nice :)

...

Dan McGee (5): Add epoch support to pacman/libalpm Update documentation to reflect new epoch package variable Make repo-add and makepkg epoch-aware Add epoch support to pactest Update contrib/ for epoch

contrib/PKGBUILD.vim | 10 ++++++++-- contrib/bacman | 3 +++ doc/PKGBUILD.5.txt | 22 ++++++++++++---------- doc/pacman.8.txt | 3 +++ lib/libalpm/alpm.h | 2 +- lib/libalpm/be_files.c | 16 +++++++++++++--- lib/libalpm/be_package.c | 2 ++ lib/libalpm/package.c | 24 ++++++++++++++---------- lib/libalpm/package.h | 2 +- scripts/makepkg.sh.in | 6 ++---- scripts/repo-add.sh.in | 7 ++++++- test/pacman/pmdb.py | 18 +++++++----------- test/pacman/pmpkg.py | 5 +++-- test/pacman/util.py | 4 ++++ 14 files changed, 79 insertions(+), 45 deletions(-)

After a quick read through the patches, it looks good to me.

Allan McRae

9 Oct 9 Oct

3:47 a.m.

I just took these for a spin.... allan@mugen /home/arch/code/pacman (epoch)

...

sudo ./src/pacman/pacman -Syu <snip>

Targets (24): blas-3.2.2-2 crafty-23.3-1 db-4.8.26-2 foomatic-db-4.0.5_20100816-1 foomatic-filters-4.0.5_20100816-1 foomatic-db-engine-4.0.5_20100816-1 ghostscript-9.00-1 gsfonts-1.0.7pre44-2 imagemagick-6.6.4.3-1 libffi-3.0.9-1 libimobiledevice-1.0.3-1 libraw1394-2.0.5-1 libshout-2.2.2-3 libvdpau-0.4-1 sqlite3-3.7.3-1 qt-4.7.0-3 rasqal-0.9.20-1 redland-1.0.11-1 soundconverter-1.5.3-4 tdb-1.2.1-2 ttf-liberation-1.06.0.20100721-1 vim-runtime-7.3.3-2 vim-7.3.3-2 vte-0.26.0-4 Total Download Size: 37.54 MB Total Installed Size: 264.40 MB ... allan@mugen /home/arch/code/pacman (epoch)

...

pacman -Syu <snip>

Targets (8): ghostscript-9.00-1 libimobiledevice-1.0.3-1 sqlite3-3.7.3-1 qt-4.7.0-3 rasqal-0.9.20-1 redland-1.0.11-1 soundconverter-1.5.3-4 vte-0.26.0-4 Total Download Size: 36.76 MB Total Installed Size: 159.49 MB Proceed with installation? [Y/n] A quick look at the extra packages using the epoch patches shows they all appear to have options=('force') and are the same version as already on my system. Allan

Xavier Chantry

12:30 p.m.

On Sat, Oct 9, 2010 at 3:47 AM, Allan McRae wrote:

...

I just took these for a spin....

allan@mugen /home/arch/code/pacman (epoch)

...
sudo ./src/pacman/pacman -Syu <snip>

Targets (24): blas-3.2.2-2 crafty-23.3-1 db-4.8.26-2 foomatic-db-4.0.5_20100816-1 foomatic-filters-4.0.5_20100816-1 foomatic-db-engine-4.0.5_20100816-1 ghostscript-9.00-1 gsfonts-1.0.7pre44-2 imagemagick-6.6.4.3-1 libffi-3.0.9-1 libimobiledevice-1.0.3-1 libraw1394-2.0.5-1 libshout-2.2.2-3 libvdpau-0.4-1 sqlite3-3.7.3-1 qt-4.7.0-3 rasqal-0.9.20-1 redland-1.0.11-1 soundconverter-1.5.3-4 tdb-1.2.1-2 ttf-liberation-1.06.0.20100721-1 vim-runtime-7.3.3-2 vim-7.3.3-2 vte-0.26.0-4

Total Download Size: 37.54 MB Total Installed Size: 264.40 MB ...

So these are all the packages on your system with force flag ? Maybe now would be a good time to review which of those really need force flag, drop it if it has been unnecessary for a while. Then if upstream mess up version again or for next need of downgrade, start using epoch. Btw this problem happens just once , right ? After this first reinstall of force package, epoch gets written to local database and then it's fine ?

Allan McRae

12:45 p.m.

On 09/10/10 20:30, Xavier Chantry wrote:

...

On Sat, Oct 9, 2010 at 3:47 AM, Allan McRae wrote:

...
I just took these for a spin....

allan@mugen /home/arch/code/pacman (epoch)

...
sudo ./src/pacman/pacman -Syu <snip>

Targets (24): blas-3.2.2-2 crafty-23.3-1 db-4.8.26-2 foomatic-db-4.0.5_20100816-1 foomatic-filters-4.0.5_20100816-1 foomatic-db-engine-4.0.5_20100816-1 ghostscript-9.00-1 gsfonts-1.0.7pre44-2 imagemagick-6.6.4.3-1 libffi-3.0.9-1 libimobiledevice-1.0.3-1 libraw1394-2.0.5-1 libshout-2.2.2-3 libvdpau-0.4-1 sqlite3-3.7.3-1 qt-4.7.0-3 rasqal-0.9.20-1 redland-1.0.11-1 soundconverter-1.5.3-4 tdb-1.2.1-2 ttf-liberation-1.06.0.20100721-1 vim-runtime-7.3.3-2 vim-7.3.3-2 vte-0.26.0-4

Total Download Size: 37.54 MB Total Installed Size: 264.40 MB ...

So these are all the packages on your system with force flag ? Maybe now would be a good time to review which of those really need force flag, drop it if it has been unnecessary for a while. Then if upstream mess up version again or for next need of downgrade, start using epoch.

Btw this problem happens just once , right ? After this first reinstall of force package, epoch gets written to local database and then it's fine ?

Dan and I had a talk about this. Note that at the moment "force" and "epoch" are not actually being written to the local database. That is actually good thing given when force is set we set epoch to MAX_INT so writing that to the local db would prevent any further upgrade... So there are things to be fixed around this. Allan

Dan McGee

12 Oct 12 Oct

12:51 a.m.

New subject: [pacman-dev] [PATCH] Read 'force' entry from packages

We weren't reading this in from our packages, thus causing us not to write it out to our local database. Adding this now will help ease the upgrade path for epoch later and not require reinstallation of all force packages. Signed-off-by: Dan McGee --- For maint, helps to address the problem Allan pointed out regarding upgrading existing packages that used the force flag. -Dan lib/libalpm/be_package.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 38cf357..ff266ae 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -75,6 +75,8 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg) STRDUP(newpkg->version, ptr, RET_ERR(PM_ERR_MEMORY, -1)); } else if(!strcmp(key, "pkgdesc")) { STRDUP(newpkg->desc, ptr, RET_ERR(PM_ERR_MEMORY, -1)); + } else if(!strcmp(key, "force")) { + newpkg->force = 1; } else if(!strcmp(key, "group")) { newpkg->groups = alpm_list_add(newpkg->groups, strdup(ptr)); } else if(!strcmp(key, "url")) { -- 1.7.3.1

Xavier Chantry

14 Oct 14 Oct

8:48 p.m.

New subject: [pacman-dev] [PATCH] Read 'force' entry from packages

On Tue, Oct 12, 2010 at 12:51 AM, Dan McGee wrote:

...

We weren't reading this in from our packages, thus causing us not to write it out to our local database. Adding this now will help ease the upgrade path for epoch later and not require reinstallation of all force packages.

Signed-off-by: Dan McGee ---

For maint, helps to address the problem Allan pointed out regarding upgrading existing packages that used the force flag.

-Dan

lib/libalpm/be_package.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 38cf357..ff266ae 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -75,6 +75,8 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg) STRDUP(newpkg->version, ptr, RET_ERR(PM_ERR_MEMORY, -1)); } else if(!strcmp(key, "pkgdesc")) { STRDUP(newpkg->desc, ptr, RET_ERR(PM_ERR_MEMORY, -1)); + } else if(!strcmp(key, "force")) { + newpkg->force = 1; } else if(!strcmp(key, "group")) { newpkg->groups = alpm_list_add(newpkg->groups, strdup(ptr)); } else if(!strcmp(key, "url")) { -- 1.7.3.1

I did not see a problem with that patch, and still don't see any, but I have some new insight after trying out git master. I think we need a similar patch in master, we still need to support old package with just force. We should do that just like sync db does (force -> epoch=1). This will ensure that epoch is always written in local db for package with force. diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ddcad59..254f830 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -184,6 +184,11 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg) STRDUP(newpkg->version, ptr, RET_ERR(PM_ERR_MEMORY, -1)); } else if(strcmp(key, "pkgdesc") == 0) { STRDUP(newpkg->desc, ptr, RET_ERR(PM_ERR_MEMORY, -1)); + } else if(strcmp(key, "force") == 0) { + /* For backward compatibility, like in sync_db_read */ + if(!newpkg->epoch) { + newpkg->epoch = 1; + } } else if(strcmp(key, "epoch") == 0) { newpkg->epoch = atoi(ptr); } else if(strcmp(key, "group") == 0) { As for the maint patch, it seems weird that we will start to write FORCE entries to local database now we decided to kill FORCE. Wouldn't it be better to start writing EPOCH so that we don't start to clutter the local db now with something which is dead ? :) On my system I just have 17 packages to reinstall, 19MB download (but 0MB download actually, thats what a download cache is for). This is done automatically on the first pacman -Su, and with above patch, the transition should be made. So I don't think we need to start writing anything to the local database now, do we ? If we still want to do that just to reduce a bit the (already small) number of packages to reinstall, what about starting to write EPOCH in maint ? I believe we just need two small changes : 1) in maint : never write FORCE in local db but write EPOCH=1 instead 2) in master : we can drop reading FORCE from local db since it never ever got written. (and EPOCH is already read) But anyway this is really a minor point, and my last proposal would actually need the 2-lines fix that got pushed to maint.

Xavier Chantry

10:39 p.m.

New subject: [pacman-dev] [PATCH] Read 'force' entry from packages

On Thu, Oct 14, 2010 at 8:48 PM, Xavier Chantry wrote:

...

On Tue, Oct 12, 2010 at 12:51 AM, Dan McGee wrote:

...
We weren't reading this in from our packages, thus causing us not to write it out to our local database. Adding this now will help ease the upgrade path for epoch later and not require reinstallation of all force packages.

Signed-off-by: Dan McGee ---

For maint, helps to address the problem Allan pointed out regarding upgrading existing packages that used the force flag.

-Dan

lib/libalpm/be_package.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 38cf357..ff266ae 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -75,6 +75,8 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg) STRDUP(newpkg->version, ptr, RET_ERR(PM_ERR_MEMORY, -1)); } else if(!strcmp(key, "pkgdesc")) { STRDUP(newpkg->desc, ptr, RET_ERR(PM_ERR_MEMORY, -1)); + } else if(!strcmp(key, "force")) { + newpkg->force = 1; } else if(!strcmp(key, "group")) { newpkg->groups = alpm_list_add(newpkg->groups, strdup(ptr)); } else if(!strcmp(key, "url")) { -- 1.7.3.1

I did not see a problem with that patch, and still don't see any, but I have some new insight after trying out git master.

I think we need a similar patch in master, we still need to support old package with just force. We should do that just like sync db does (force -> epoch=1). This will ensure that epoch is always written in local db for package with force.

diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ddcad59..254f830 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -184,6 +184,11 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg) STRDUP(newpkg->version, ptr, RET_ERR(PM_ERR_MEMORY, -1)); } else if(strcmp(key, "pkgdesc") == 0) { STRDUP(newpkg->desc, ptr, RET_ERR(PM_ERR_MEMORY, -1)); + } else if(strcmp(key, "force") == 0) { + /* For backward compatibility, like in sync_db_read */ + if(!newpkg->epoch) { + newpkg->epoch = 1; + } } else if(strcmp(key, "epoch") == 0) { newpkg->epoch = atoi(ptr); } else if(strcmp(key, "group") == 0) {

As for the maint patch, it seems weird that we will start to write FORCE entries to local database now we decided to kill FORCE. Wouldn't it be better to start writing EPOCH so that we don't start to clutter the local db now with something which is dead ? :)

On my system I just have 17 packages to reinstall, 19MB download (but 0MB download actually, thats what a download cache is for). This is done automatically on the first pacman -Su, and with above patch, the transition should be made. So I don't think we need to start writing anything to the local database now, do we ?

If we still want to do that just to reduce a bit the (already small) number of packages to reinstall, what about starting to write EPOCH in maint ? I believe we just need two small changes : 1) in maint : never write FORCE in local db but write EPOCH=1 instead 2) in master : we can drop reading FORCE from local db since it never ever got written. (and EPOCH is already read)

But anyway this is really a minor point, and my last proposal would actually need the 2-lines fix that got pushed to maint.

I just pushed this work, 3 small patches. http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=epoch-maint * be_files: write EPOCH instead of FORCE http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=epoch-master * be_package: read force entry and convert to epoch * be_local: drop FORCE in db_read

Dan McGee

15 Oct 15 Oct

3:15 a.m.

New subject: [pacman-dev] [PATCH] Read 'force' entry from packages

On Thu, Oct 14, 2010 at 3:39 PM, Xavier Chantry wrote:

...

On Thu, Oct 14, 2010 at 8:48 PM, Xavier Chantry wrote:

...
On Tue, Oct 12, 2010 at 12:51 AM, Dan McGee wrote:

...
We weren't reading this in from our packages, thus causing us not to write it out to our local database. Adding this now will help ease the upgrade path for epoch later and not require reinstallation of all force packages.

I did not see a problem with that patch, and still don't see any, but I have some new insight after trying out git master.

I think we need a similar patch in master, we still need to support old package with just force. We should do that just like sync db does (force -> epoch=1). This will ensure that epoch is always written in local db for package with force.

You are correct, we do need this.

...

...
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ddcad59..254f830 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -184,6 +184,11 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg) STRDUP(newpkg->version, ptr, RET_ERR(PM_ERR_MEMORY, -1)); } else if(strcmp(key, "pkgdesc") == 0) { STRDUP(newpkg->desc, ptr, RET_ERR(PM_ERR_MEMORY, -1)); + } else if(strcmp(key, "force") == 0) { + /* For backward compatibility, like in sync_db_read */ + if(!newpkg->epoch) { + newpkg->epoch = 1; + } } else if(strcmp(key, "epoch") == 0) { newpkg->epoch = atoi(ptr); } else if(strcmp(key, "group") == 0) {

As for the maint patch, it seems weird that we will start to write FORCE entries to local database now we decided to kill FORCE. Wouldn't it be better to start writing EPOCH so that we don't start to clutter the local db now with something which is dead ? :)

No, because we are then half-introducing a feature to a maint branch where I want as little breakage as possible, and also a database that doesn't require a lot of extra parsing code.

...

...
On my system I just have 17 packages to reinstall, 19MB download (but 0MB download actually, thats what a download cache is for). This is done automatically on the first pacman -Su, and with above patch, the transition should be made. So I don't think we need to start writing anything to the local database now, do we ?

If we still want to do that just to reduce a bit the (already small) number of packages to reinstall, what about starting to write EPOCH in maint ? I believe we just need two small changes : 1) in maint : never write FORCE in local db but write EPOCH=1 instead 2) in master : we can drop reading FORCE from local db since it never ever got written. (and EPOCH is already read)

But anyway this is really a minor point, and my last proposal would actually need the 2-lines fix that got pushed to maint.

I just pushed this work, 3 small patches.

http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=epoch-maint * be_files: write EPOCH instead of FORCE http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=epoch-master * be_package: read force entry and convert to epoch * be_local: drop FORCE in db_read

So yes, I'm a bit torn here, but I always fall on the "let's not touch maint more than necessary" side of the fence, and this starts to push that line. I'm fine with the "read force from package" patch, but the rest I'm just not so keen on. -Dan

Xavier Chantry

11:23 a.m.

New subject: [pacman-dev] [PATCH] Read 'force' entry from packages

On Fri, Oct 15, 2010 at 3:15 AM, Dan McGee wrote:

...

So yes, I'm a bit torn here, but I always fall on the "let's not touch maint more than necessary" side of the fence, and this starts to push that line. I'm fine with the "read force from package" patch, but the rest I'm just not so keen on.

As I said, if we want to be strict on the "let's not touch maint more than necessary", we can drop your patch too. It's not necessary, is it ? It will save 10 seconds on one and only one -Su operation. If not, my additional patch is not a half-feature, it's an one-line change in a completely dead and unused codepath (before your patch). Its consequences are also simpler to appreciate : we just write EPOCH instead of FORCE, and db_read simply ignores unknown lines. Your first patch is less obvious : we now set pkg->force when loading a package from its file, does it really have no impact anywhere in the codebase besides db_write ? (Looks fine.) The only purpose of these two proposals (dropping your patch or adding mine) is to provide an answer to the question 'When can we drop reading FORCE from local db over which we have no real control?' and the answer is 'right now'. Ok this is a very minor benefit, but you should give me at least an equally minor drawback for each of the proposal before rejecting them.

4952

Age (days ago)

4959

Last active (days ago)

List overview

Download

16 comments

4 participants

participants (4)

Allan McRae
Dan McGee
Dan McGee
Xavier Chantry