From nobody@FreeBSD.org Sun Mar 28 08:44:43 2010 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BB2B31065677 for ; Sun, 28 Mar 2010 08:44:43 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id A8C618FC14 for ; Sun, 28 Mar 2010 08:44:43 +0000 (UTC) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id o2S8ihOw007801 for ; Sun, 28 Mar 2010 08:44:43 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id o2S8ihqt007800; Sun, 28 Mar 2010 08:44:43 GMT (envelope-from nobody) Message-Id: <201003280844.o2S8ihqt007800@www.freebsd.org> Date: Sun, 28 Mar 2010 08:44:43 GMT From: Garrett Cooper To: freebsd-gnats-submit@FreeBSD.org Subject: [patch] pkg_add(1) - remove hardcoded versioning data from add/main.c X-Send-Pr-Version: www-3.1 X-GNATS-Notify: >Number: 145100 >Category: bin >Synopsis: [patch] pkg_add(1) - remove hardcoded versioning data from add/main.c >Confidential: no >Severity: non-critical >Priority: medium >Responsible: portmgr >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sun Mar 28 08:50:02 UTC 2010 >Closed-Date: >Last-Modified: Mon Jan 02 15:27:23 UTC 2012 >Originator: Garrett Cooper >Release: 9-CURRENT >Organization: Cisco Systems, Inc. >Environment: FreeBSD bayonetta.local 9.0-CURRENT FreeBSD 9.0-CURRENT #5 r205310: Sat Mar 20 01:32:51 PDT 2010 gcooper@bayonetta.local:/usr/obj/usr/src/sys/BAYONETTA amd64 >Description: A keen observation made by bapt@ on #bsdports when submitting the patch for pkg_add, was the fact that the hardcoded versioning information in add/main.c was useless in lieu of (struct uname).release's data. We'd be losing a small potential feature in the event that ABI breakage was to ever occur (say with the syscons utmp -> utmpx removal), but that shouldn't occur on a regular basis and should only occur on CURRENT (where packages should be built by the end user anyhow IMO..). Besides, if the admin knows enough about the system he / she should be capable enough of determining whether or not he / she should be using FreeBSD version X.Y.Z. This eases the amount of flux with the folks in re@ and although it could have been circumvented via the environment variable - OSRELDATE - it's just simpler to do things this way (via UNAME_r). If committed, this item will need an announcement on current and stable beforehand to ensure that all affected parties are aware of the change and update their infrastructures accordingly. >How-To-Repeat: >Fix: Patch attached with submission follows: ==== //depot/projects/soc2007/gcooper-pkg_install-enhancements-simplified/usr.sbin/pkg_install/add/main.c#3 - /scratch/freebsd/perforce/pkg_install-enhancements/usr.sbin/pkg_install/add/main.c ==== @@ -52,51 +52,6 @@ char *progname = NULL; -struct { - int lowver; /* Lowest version number to match */ - int hiver; /* Highest version number to match */ - const char *directory; /* Directory it lives in */ -} releases[] = { - { 410000, 410000, "/packages-4.1-release" }, - { 420000, 420000, "/packages-4.2-release" }, - { 430000, 430000, "/packages-4.3-release" }, - { 440000, 440000, "/packages-4.4-release" }, - { 450000, 450000, "/packages-4.5-release" }, - { 460000, 460001, "/packages-4.6-release" }, - { 460002, 460099, "/packages-4.6.2-release" }, - { 470000, 470099, "/packages-4.7-release" }, - { 480000, 480099, "/packages-4.8-release" }, - { 490000, 490099, "/packages-4.9-release" }, - { 491000, 491099, "/packages-4.10-release" }, - { 492000, 492099, "/packages-4.11-release" }, - { 500000, 500099, "/packages-5.0-release" }, - { 501000, 501099, "/packages-5.1-release" }, - { 502000, 502009, "/packages-5.2-release" }, - { 502010, 502099, "/packages-5.2.1-release" }, - { 503000, 503099, "/packages-5.3-release" }, - { 504000, 504099, "/packages-5.4-release" }, - { 505000, 505099, "/packages-5.5-release" }, - { 600000, 600099, "/packages-6.0-release" }, - { 601000, 601099, "/packages-6.1-release" }, - { 602000, 602099, "/packages-6.2-release" }, - { 603000, 603099, "/packages-6.3-release" }, - { 604000, 604099, "/packages-6.4-release" }, - { 700000, 700099, "/packages-7.0-release" }, - { 701000, 701099, "/packages-7.1-release" }, - { 702000, 702099, "/packages-7.2-release" }, - { 800000, 800499, "/packages-8.0-release" }, - { 300000, 399000, "/packages-3-stable" }, - { 400000, 499000, "/packages-4-stable" }, - { 502100, 502128, "/packages-5-current" }, - { 503100, 599000, "/packages-5-stable" }, - { 600100, 699000, "/packages-6-stable" }, - { 700100, 799000, "/packages-7-stable" }, - { 800500, 899000, "/packages-8-stable" }, - { 900000, 999000, "/packages-9-current" }, - { 0, 9999999, "/packages-current" }, - { 0, 0, NULL } -}; - static char *getpackagesite(void); int getosreldate(void); @@ -302,8 +257,8 @@ static char * getpackagesite(void) { - int reldate, i; static char sitepath[MAXPATHLEN]; + size_t i; struct utsname u; if (getenv("PACKAGESITE")) { @@ -327,20 +282,22 @@ >= sizeof(sitepath)) return NULL; - uname(&u); - if (strlcat(sitepath, u.machine, sizeof(sitepath)) >= sizeof(sitepath)) + if (uname(&u) == -1) { + warn("%s.%s: could not determine uname information", progname, + __func__); + return NULL; + } + if (strlcat(sitepath, u.machine, sizeof(sitepath)) >= sizeof(sitepath) || + strlcat(sitepath, "/", sizeof(sitepath)) >= sizeof(sitepath)) return NULL; - reldate = getosreldate(); - for(i = 0; releases[i].directory != NULL; i++) { - if (reldate >= releases[i].lowver && reldate <= releases[i].hiver) { - if (strlcat(sitepath, releases[i].directory, sizeof(sitepath)) - >= sizeof(sitepath)) - return NULL; - break; - } + for (i = 0; u.release[i] != '\0'; i++) { + u.release[i] = tolower(u.release[i]); } + if (strlcat(sitepath, u.release, sizeof(sitepath)) >= sizeof(sitepath)) + return NULL; + if (strlcat(sitepath, "/Latest/", sizeof(sitepath)) >= sizeof(sitepath)) return NULL; >Release-Note: >Audit-Trail: From: Garrett Cooper To: FreeBSD-gnats-submit@freebsd.org, freebsd-bugs@freebsd.org Cc: Subject: Re: bin/145100: [patch] pkg_add(1) - remove hardcoded versioning data from add/main.c Date: Sun, 28 Mar 2010 02:13:57 -0700 --0016e68ee285ff239c0482d8d0ac Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On Sun, Mar 28, 2010 at 1:50 AM, wrote: > Thank you very much for your problem report. > It has the internal identification `bin/145100'. > The individual assigned to look at your > report is: freebsd-bugs. > > You can access the state of your problem report at any time > via this link: > > http://www.freebsd.org/cgi/query-pr.cgi?pr=3D145100 > >>Category: =A0 =A0 =A0 bin >>Responsible: =A0 =A0freebsd-bugs >>Synopsis: =A0 =A0 =A0 [patch] pkg_add(1) - remove hardcoded versioning da= ta from add/main.c >>Arrival-Date: =A0 Sun Mar 28 08:50:02 UTC 2010 Supported hierarchies are done like: //packages- Corrected with this diff. Thanks, -Garrett --0016e68ee285ff239c0482d8d0ac Content-Type: text/plain; charset=US-ASCII; name="simplify-pkg-install-versioning.diff.txt" Content-Disposition: attachment; filename="simplify-pkg-install-versioning.diff.txt" Content-Transfer-Encoding: base64 X-Attachment-Id: f_g7bmugbi0 PT09PSAvL2RlcG90L3Byb2plY3RzL3NvYzIwMDcvZ2Nvb3Blci1wa2dfaW5zdGFsbC1lbmhhbmNl bWVudHMtc2ltcGxpZmllZC91c3Iuc2Jpbi9wa2dfaW5zdGFsbC9hZGQvbWFpbi5jIzMgLSAvc2Ny YXRjaC9mcmVlYnNkL3BlcmZvcmNlL3BrZ19pbnN0YWxsLWVuaGFuY2VtZW50cy91c3Iuc2Jpbi9w a2dfaW5zdGFsbC9hZGQvbWFpbi5jID09PT0KQEAgLTUyLDUxICs1Miw2IEBACiAKIGNoYXIJKnBy b2duYW1lCT0gTlVMTDsKIAotc3RydWN0IHsKLQlpbnQgbG93dmVyOwkvKiBMb3dlc3QgdmVyc2lv biBudW1iZXIgdG8gbWF0Y2ggKi8KLQlpbnQgaGl2ZXI7CS8qIEhpZ2hlc3QgdmVyc2lvbiBudW1i ZXIgdG8gbWF0Y2ggKi8KLQljb25zdCBjaGFyICpkaXJlY3Rvcnk7CS8qIERpcmVjdG9yeSBpdCBs aXZlcyBpbiAqLwotfSByZWxlYXNlc1tdID0gewotCXsgNDEwMDAwLCA0MTAwMDAsICIvcGFja2Fn ZXMtNC4xLXJlbGVhc2UiIH0sCi0JeyA0MjAwMDAsIDQyMDAwMCwgIi9wYWNrYWdlcy00LjItcmVs ZWFzZSIgfSwKLQl7IDQzMDAwMCwgNDMwMDAwLCAiL3BhY2thZ2VzLTQuMy1yZWxlYXNlIiB9LAot CXsgNDQwMDAwLCA0NDAwMDAsICIvcGFja2FnZXMtNC40LXJlbGVhc2UiIH0sCi0JeyA0NTAwMDAs IDQ1MDAwMCwgIi9wYWNrYWdlcy00LjUtcmVsZWFzZSIgfSwKLQl7IDQ2MDAwMCwgNDYwMDAxLCAi L3BhY2thZ2VzLTQuNi1yZWxlYXNlIiB9LAotCXsgNDYwMDAyLCA0NjAwOTksICIvcGFja2FnZXMt NC42LjItcmVsZWFzZSIgfSwKLQl7IDQ3MDAwMCwgNDcwMDk5LCAiL3BhY2thZ2VzLTQuNy1yZWxl YXNlIiB9LAotCXsgNDgwMDAwLCA0ODAwOTksICIvcGFja2FnZXMtNC44LXJlbGVhc2UiIH0sCi0J eyA0OTAwMDAsIDQ5MDA5OSwgIi9wYWNrYWdlcy00LjktcmVsZWFzZSIgfSwKLQl7IDQ5MTAwMCwg NDkxMDk5LCAiL3BhY2thZ2VzLTQuMTAtcmVsZWFzZSIgfSwKLQl7IDQ5MjAwMCwgNDkyMDk5LCAi L3BhY2thZ2VzLTQuMTEtcmVsZWFzZSIgfSwKLQl7IDUwMDAwMCwgNTAwMDk5LCAiL3BhY2thZ2Vz LTUuMC1yZWxlYXNlIiB9LAotCXsgNTAxMDAwLCA1MDEwOTksICIvcGFja2FnZXMtNS4xLXJlbGVh c2UiIH0sCi0JeyA1MDIwMDAsIDUwMjAwOSwgIi9wYWNrYWdlcy01LjItcmVsZWFzZSIgfSwKLQl7 IDUwMjAxMCwgNTAyMDk5LCAiL3BhY2thZ2VzLTUuMi4xLXJlbGVhc2UiIH0sCi0JeyA1MDMwMDAs IDUwMzA5OSwgIi9wYWNrYWdlcy01LjMtcmVsZWFzZSIgfSwKLQl7IDUwNDAwMCwgNTA0MDk5LCAi L3BhY2thZ2VzLTUuNC1yZWxlYXNlIiB9LAotCXsgNTA1MDAwLCA1MDUwOTksICIvcGFja2FnZXMt NS41LXJlbGVhc2UiIH0sCi0JeyA2MDAwMDAsIDYwMDA5OSwgIi9wYWNrYWdlcy02LjAtcmVsZWFz ZSIgfSwKLQl7IDYwMTAwMCwgNjAxMDk5LCAiL3BhY2thZ2VzLTYuMS1yZWxlYXNlIiB9LAotCXsg NjAyMDAwLCA2MDIwOTksICIvcGFja2FnZXMtNi4yLXJlbGVhc2UiIH0sCi0JeyA2MDMwMDAsIDYw MzA5OSwgIi9wYWNrYWdlcy02LjMtcmVsZWFzZSIgfSwKLQl7IDYwNDAwMCwgNjA0MDk5LCAiL3Bh Y2thZ2VzLTYuNC1yZWxlYXNlIiB9LAotCXsgNzAwMDAwLCA3MDAwOTksICIvcGFja2FnZXMtNy4w LXJlbGVhc2UiIH0sCi0JeyA3MDEwMDAsIDcwMTA5OSwgIi9wYWNrYWdlcy03LjEtcmVsZWFzZSIg fSwKLQl7IDcwMjAwMCwgNzAyMDk5LCAiL3BhY2thZ2VzLTcuMi1yZWxlYXNlIiB9LAotCXsgODAw MDAwLCA4MDA0OTksICIvcGFja2FnZXMtOC4wLXJlbGVhc2UiIH0sCi0JeyAzMDAwMDAsIDM5OTAw MCwgIi9wYWNrYWdlcy0zLXN0YWJsZSIgfSwKLQl7IDQwMDAwMCwgNDk5MDAwLCAiL3BhY2thZ2Vz LTQtc3RhYmxlIiB9LAotCXsgNTAyMTAwLCA1MDIxMjgsICIvcGFja2FnZXMtNS1jdXJyZW50IiB9 LAotCXsgNTAzMTAwLCA1OTkwMDAsICIvcGFja2FnZXMtNS1zdGFibGUiIH0sCi0JeyA2MDAxMDAs IDY5OTAwMCwgIi9wYWNrYWdlcy02LXN0YWJsZSIgfSwKLQl7IDcwMDEwMCwgNzk5MDAwLCAiL3Bh Y2thZ2VzLTctc3RhYmxlIiB9LAotCXsgODAwNTAwLCA4OTkwMDAsICIvcGFja2FnZXMtOC1zdGFi bGUiIH0sCi0JeyA5MDAwMDAsIDk5OTAwMCwgIi9wYWNrYWdlcy05LWN1cnJlbnQiIH0sCi0JeyAw LCA5OTk5OTk5LCAiL3BhY2thZ2VzLWN1cnJlbnQiIH0sCi0JeyAwLCAwLCBOVUxMIH0KLX07Ci0K IHN0YXRpYyBjaGFyICpnZXRwYWNrYWdlc2l0ZSh2b2lkKTsKIGludCBnZXRvc3JlbGRhdGUodm9p ZCk7CiAKQEAgLTMwMiw4ICsyNTcsOCBAQAogc3RhdGljIGNoYXIgKgogZ2V0cGFja2FnZXNpdGUo dm9pZCkKIHsKLSAgICBpbnQgcmVsZGF0ZSwgaTsKICAgICBzdGF0aWMgY2hhciBzaXRlcGF0aFtN QVhQQVRITEVOXTsKKyAgICBzaXplX3QgaTsKICAgICBzdHJ1Y3QgdXRzbmFtZSB1OwogCiAgICAg aWYgKGdldGVudigiUEFDS0FHRVNJVEUiKSkgewpAQCAtMzI3LDIwICsyODIsMjIgQEAKIAk+PSBz aXplb2Yoc2l0ZXBhdGgpKQogCXJldHVybiBOVUxMOwogCi0gICAgdW5hbWUoJnUpOwotICAgIGlm IChzdHJsY2F0KHNpdGVwYXRoLCB1Lm1hY2hpbmUsIHNpemVvZihzaXRlcGF0aCkpID49IHNpemVv ZihzaXRlcGF0aCkpCisgICAgaWYgKHVuYW1lKCZ1KSA9PSAtMSkgeworCXdhcm4oIiVzLiVzOiBj b3VsZCBub3QgZGV0ZXJtaW5lIHVuYW1lIGluZm9ybWF0aW9uIiwgcHJvZ25hbWUsCisJICAgIF9f ZnVuY19fKTsKKwlyZXR1cm4gTlVMTDsKKyAgICB9CisgICAgaWYgKHN0cmxjYXQoc2l0ZXBhdGgs IHUubWFjaGluZSwgc2l6ZW9mKHNpdGVwYXRoKSkgPj0gc2l6ZW9mKHNpdGVwYXRoKSB8fAorICAg ICAgICBzdHJsY2F0KHNpdGVwYXRoLCAiL3BhY2thZ2VzLSIsIHNpemVvZihzaXRlcGF0aCkpID49 IHNpemVvZihzaXRlcGF0aCkpCiAJcmV0dXJuIE5VTEw7CiAKLSAgICByZWxkYXRlID0gZ2V0b3Ny ZWxkYXRlKCk7Ci0gICAgZm9yKGkgPSAwOyByZWxlYXNlc1tpXS5kaXJlY3RvcnkgIT0gTlVMTDsg aSsrKSB7Ci0JaWYgKHJlbGRhdGUgPj0gcmVsZWFzZXNbaV0ubG93dmVyICYmIHJlbGRhdGUgPD0g cmVsZWFzZXNbaV0uaGl2ZXIpIHsKLQkgICAgaWYgKHN0cmxjYXQoc2l0ZXBhdGgsIHJlbGVhc2Vz W2ldLmRpcmVjdG9yeSwgc2l6ZW9mKHNpdGVwYXRoKSkKLQkJPj0gc2l6ZW9mKHNpdGVwYXRoKSkK LQkJcmV0dXJuIE5VTEw7Ci0JICAgIGJyZWFrOwotCX0KKyAgICBmb3IgKGkgPSAwOyB1LnJlbGVh c2VbaV0gIT0gJ1wwJzsgaSsrKSB7CisgICAgICAgIHUucmVsZWFzZVtpXSA9IHRvbG93ZXIodS5y ZWxlYXNlW2ldKTsKICAgICB9CiAKKyAgICBpZiAoc3RybGNhdChzaXRlcGF0aCwgdS5yZWxlYXNl LCBzaXplb2Yoc2l0ZXBhdGgpKSA+PSBzaXplb2Yoc2l0ZXBhdGgpKQorCXJldHVybiBOVUxMOwor CiAgICAgaWYgKHN0cmxjYXQoc2l0ZXBhdGgsICIvTGF0ZXN0LyIsIHNpemVvZihzaXRlcGF0aCkp ID49IHNpemVvZihzaXRlcGF0aCkpCiAJcmV0dXJuIE5VTEw7CiAK --0016e68ee285ff239c0482d8d0ac-- From: Garrett Cooper To: Garrett Cooper Cc: FreeBSD-gnats-submit@freebsd.org, freebsd-bugs@freebsd.org Subject: Re: bin/145100: [patch] pkg_add(1) - remove hardcoded versioning data from add/main.c Date: Sun, 28 Mar 2010 02:17:17 -0700 On Sun, Mar 28, 2010 at 2:13 AM, Garrett Cooper wrote= : > On Sun, Mar 28, 2010 at 1:50 AM, =A0 wr= ote: >> Thank you very much for your problem report. >> It has the internal identification `bin/145100'. >> The individual assigned to look at your >> report is: freebsd-bugs. >> >> You can access the state of your problem report at any time >> via this link: >> >> http://www.freebsd.org/cgi/query-pr.cgi?pr=3D145100 >> >>>Category: =A0 =A0 =A0 bin >>>Responsible: =A0 =A0freebsd-bugs >>>Synopsis: =A0 =A0 =A0 [patch] pkg_add(1) - remove hardcoded versioning d= ata from add/main.c >>>Arrival-Date: =A0 Sun Mar 28 08:50:02 UTC 2010 > > Supported hierarchies are done like: > > =A0 =A0//packages- > > Corrected with this diff. One other minor sidenote: this patch requires minor a basename(3) addition to pkg_add(1) as described in bin/121165 . It's relatively trivial to add, and only needs to be done for lib/lib.h and add/main.c ; so either I can yank the diagnostic message, or add the minor change to the diff -- whichever is more preferred. Thanks, -Garrett From: Garrett Cooper To: Garrett Cooper Cc: bug-followup@freebsd.org, freebsd-bugs@freebsd.org Subject: Re: bin/145100: [patch] pkg_add(1) - remove hardcoded versioning data from add/main.c Date: Sun, 28 Mar 2010 12:07:07 -0700 Hi Ken, On Sun, Mar 28, 2010 at 2:17 AM, Garrett Cooper wrote= : > On Sun, Mar 28, 2010 at 2:13 AM, Garrett Cooper wro= te: >> On Sun, Mar 28, 2010 at 1:50 AM, =A0 w= rote: >>> Thank you very much for your problem report. >>> It has the internal identification `bin/145100'. >>> The individual assigned to look at your >>> report is: freebsd-bugs. >>> >>> You can access the state of your problem report at any time >>> via this link: >>> >>> http://www.freebsd.org/cgi/query-pr.cgi?pr=3D145100 >>> >>>>Category: =A0 =A0 =A0 bin >>>>Responsible: =A0 =A0freebsd-bugs >>>>Synopsis: =A0 =A0 =A0 [patch] pkg_add(1) - remove hardcoded versioning = data from add/main.c >>>>Arrival-Date: =A0 Sun Mar 28 08:50:02 UTC 2010 >> >> Supported hierarchies are done like: >> >> =A0 =A0//packages- >> >> Corrected with this diff. > > =A0 =A0One other minor sidenote: this patch requires minor a basename(3) > addition to pkg_add(1) as described in bin/121165 . It's relatively > trivial to add, and only needs to be done for lib/lib.h and add/main.c > ; so either I can yank the diagnostic message, or add the minor change > to the diff -- whichever is more preferred. > There are a couple of issues this patch doesn't seen to address. > Here is an example of what's in the uname structure on a machine > that's had two patches applied to it (SA/EN as published by the > Security Team): > > bauer 11 % cat uname.c > #include > #include > #include > > int > main(int argc, char *argv[]) > { > struct utsname un; > > if (uname(&un)) { > perror("uname"); > exit (1); > } > printf("sysname: %s\n", un.sysname); > printf("nodename: %s\n", un.nodename); > printf("release: %s\n", un.release); > printf("version: %s\n", un.version); > printf("machine: %s\n", un.machine); > } > bauer 12 % ./uname > sysname: FreeBSD > nodename: bauer.cse.buffalo.edu > release: 8.0-RELEASE-p2 > version: FreeBSD 8.0-RELEASE-p2 #0: Fri Mar 26 16:58:16 EDT 2010 > root@bauer.cse.buffalo.edu:/usr/obj/usr/src/sys/FIREWALL > machine: amd64 > bauer 13 % > > So unless I'm mis-reading your patch it would be looking for > packages in > > /ftp/pub/FreeBSD/ports/amd64/packages-8.0-release-p2 > > which doesn't exist. > > That problem isn't too hard to solve but the other problem is. > There are times during release cycles that branches wind up > with even weirder names than just tacking -p on to > the end of the name. For example during the 7.3 release cycle > the stable/7 branch was named 7.3-PRERELEASE during the entire > cycle. Once it got created the releng/7.3 branch was named > 7.3-RC1, and progressed to 7.3-RC2. And take a look at what > a system installed from one of the Monthly Snapshots gives for > uname output, I don't have one handy at the moment but if I > recall correctly it has the snapshot's name embedded in the > uname output. The mechanism that does that is what I use to > name the BETA releases as well, I never actually commit the > BETA1, BETA2, etc. names to a stable branch because it tends > to freak out people using those branches (we wind up getting > mail saying "Hey, RELENG_7 is a stable branch! Why does > a machine updated today on RELENG_7 say it's *BETA1*???") > during release cycles; the PRERELEASE thing is an attempt > to avoid that...). If you do a release build specifying > BUILDNAME on the command line it will use that as what gets > put into sys/conf/newvers.sh as the $RELEASE. And that's > the source of what uname gives as the release field. Ouch. You pointed out a flaw in my assumptions that would definitely invalidate this proposed change. Now I'm teetering between whether or not it's wise to actually make this change. Here are some questions though: 1. What happens if compat libraries are used with a specifically built copy of pkg_install? Game over, right -- because the __FreeBSD_version is encoded in the binary? 2. Should prereleases really be allowed to use release-based packages? Probably not right -- generally the functionality is fixed in each release, but it can change dramatically before the official release is made, correct (take the 7.0-RELEASE for example...)? 3. What also happens if FreeBSD developer goes and messes up a package before the release 7.2-RC2, but it was working in 7.2-RC1 -- the individual will be toast right because they'll `automatically upgrade' to the latest version and can't go back to the earlier version without grabbing the CD, correct? Thanks, -Garrett From: Garrett Cooper To: Garrett Cooper Cc: bug-followup@freebsd.org, freebsd-bugs@freebsd.org, kensmith@freebsd.org Subject: Re: bin/145100: [patch] pkg_add(1) - remove hardcoded versioning data from add/main.c Date: Sun, 28 Mar 2010 12:08:07 -0700 On Sun, Mar 28, 2010 at 12:07 PM, Garrett Cooper wrot= e: > Hi Ken, > > On Sun, Mar 28, 2010 at 2:17 AM, Garrett Cooper wro= te: >> On Sun, Mar 28, 2010 at 2:13 AM, Garrett Cooper wr= ote: >>> On Sun, Mar 28, 2010 at 1:50 AM, =A0 = wrote: >>>> Thank you very much for your problem report. >>>> It has the internal identification `bin/145100'. >>>> The individual assigned to look at your >>>> report is: freebsd-bugs. >>>> >>>> You can access the state of your problem report at any time >>>> via this link: >>>> >>>> http://www.freebsd.org/cgi/query-pr.cgi?pr=3D145100 >>>> >>>>>Category: =A0 =A0 =A0 bin >>>>>Responsible: =A0 =A0freebsd-bugs >>>>>Synopsis: =A0 =A0 =A0 [patch] pkg_add(1) - remove hardcoded versioning= data from add/main.c >>>>>Arrival-Date: =A0 Sun Mar 28 08:50:02 UTC 2010 >>> >>> Supported hierarchies are done like: >>> >>> =A0 =A0//packages- >>> >>> Corrected with this diff. >> >> =A0 =A0One other minor sidenote: this patch requires minor a basename(3) >> addition to pkg_add(1) as described in bin/121165 . It's relatively >> trivial to add, and only needs to be done for lib/lib.h and add/main.c >> ; so either I can yank the diagnostic message, or add the minor change >> to the diff -- whichever is more preferred. ---- >> There are a couple of issues this patch doesn't seen to address. >> Here is an example of what's in the uname structure on a machine >> that's had two patches applied to it (SA/EN as published by the >> Security Team): >> >> bauer 11 % cat uname.c >> #include >> #include >> #include >> >> int >> main(int argc, char *argv[]) >> { >> =A0 =A0 =A0 =A0struct utsname un; >> >> =A0 =A0 =A0 =A0if (uname(&un)) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0perror("uname"); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0exit (1); >> =A0 =A0 =A0 =A0} >> =A0 =A0 =A0 =A0printf("sysname: %s\n", un.sysname); >> =A0 =A0 =A0 =A0printf("nodename: %s\n", un.nodename); >> =A0 =A0 =A0 =A0printf("release: %s\n", un.release); >> =A0 =A0 =A0 =A0printf("version: %s\n", un.version); >> =A0 =A0 =A0 =A0printf("machine: %s\n", un.machine); >> } >> bauer 12 % ./uname >> sysname: FreeBSD >> nodename: bauer.cse.buffalo.edu >> release: 8.0-RELEASE-p2 >> version: FreeBSD 8.0-RELEASE-p2 #0: Fri Mar 26 16:58:16 EDT 2010 >> root@bauer.cse.buffalo.edu:/usr/obj/usr/src/sys/FIREWALL >> machine: amd64 >> bauer 13 % >> >> So unless I'm mis-reading your patch it would be looking for >> packages in >> >> =A0/ftp/pub/FreeBSD/ports/amd64/packages-8.0-release-p2 >> >> which doesn't exist. >> >> That problem isn't too hard to solve but the other problem is. >> There are times during release cycles that branches wind up >> with even weirder names than just tacking -p on to >> the end of the name. =A0For example during the 7.3 release cycle >> the stable/7 branch was named 7.3-PRERELEASE during the entire >> cycle. =A0Once it got created the releng/7.3 branch was named >> 7.3-RC1, and progressed to 7.3-RC2. =A0And take a look at what >> a system installed from one of the Monthly Snapshots gives for >> uname output, I don't have one handy at the moment but if I >> recall correctly it has the snapshot's name embedded in the >> uname output. =A0The mechanism that does that is what I use to >> name the BETA releases as well, I never actually commit the >> BETA1, BETA2, etc. names to a stable branch because it tends >> to freak out people using those branches (we wind up getting >> mail saying "Hey, RELENG_7 is a stable branch! =A0Why does >> a machine updated today on RELENG_7 say it's *BETA1*???") >> during release cycles; the PRERELEASE thing is an attempt >> to avoid that...). =A0If you do a release build specifying >> BUILDNAME on the command line it will use that as what gets >> put into sys/conf/newvers.sh as the $RELEASE. =A0And that's >> the source of what uname gives as the release field. > > =A0 =A0Ouch. You pointed out a flaw in my assumptions that would > definitely invalidate this proposed change. Now I'm teetering between > whether or not it's wise to actually make this change. > > =A0 =A0Here are some questions though: > > 1. What happens if compat libraries are used with a specifically built > copy of pkg_install? Game over, right -- because the __FreeBSD_version > is encoded in the binary? > 2. Should prereleases really be allowed to use release-based packages? > Probably not right -- generally the functionality is fixed in each > release, but it can change dramatically before the official release is > made, correct (take the 7.0-RELEASE for example...)? > 3. What also happens if FreeBSD developer goes and messes up a package > before the release 7.2-RC2, but it was working in 7.2-RC1 -- the > individual will be toast right because they'll `automatically upgrade' > to the latest version and can't go back to the earlier version without > grabbing the CD, correct? Forgot to actually CC ken :/... -Garrett From: Garrett Cooper To: Garrett Cooper Cc: bug-followup@freebsd.org, freebsd-bugs@freebsd.org, kensmith@freebsd.org Subject: Re: bin/145100: [patch] pkg_add(1) - remove hardcoded versioning data from add/main.c Date: Sun, 28 Mar 2010 15:42:47 -0700 On Sun, Mar 28, 2010 at 12:08 PM, Garrett Cooper wrot= e: > On Sun, Mar 28, 2010 at 12:07 PM, Garrett Cooper wr= ote: >> Hi Ken, >> >> On Sun, Mar 28, 2010 at 2:17 AM, Garrett Cooper wr= ote: >>> On Sun, Mar 28, 2010 at 2:13 AM, Garrett Cooper w= rote: >>>> On Sun, Mar 28, 2010 at 1:50 AM, =A0= wrote: >>>>> Thank you very much for your problem report. >>>>> It has the internal identification `bin/145100'. >>>>> The individual assigned to look at your >>>>> report is: freebsd-bugs. >>>>> >>>>> You can access the state of your problem report at any time >>>>> via this link: >>>>> >>>>> http://www.freebsd.org/cgi/query-pr.cgi?pr=3D145100 >>>>> >>>>>>Category: =A0 =A0 =A0 bin >>>>>>Responsible: =A0 =A0freebsd-bugs >>>>>>Synopsis: =A0 =A0 =A0 [patch] pkg_add(1) - remove hardcoded versionin= g data from add/main.c >>>>>>Arrival-Date: =A0 Sun Mar 28 08:50:02 UTC 2010 >>>> >>>> Supported hierarchies are done like: >>>> >>>> =A0 =A0//packages- >>>> >>>> Corrected with this diff. >>> >>> =A0 =A0One other minor sidenote: this patch requires minor a basename(3= ) >>> addition to pkg_add(1) as described in bin/121165 . It's relatively >>> trivial to add, and only needs to be done for lib/lib.h and add/main.c >>> ; so either I can yank the diagnostic message, or add the minor change >>> to the diff -- whichever is more preferred. > > ---- > >>> There are a couple of issues this patch doesn't seen to address. >>> Here is an example of what's in the uname structure on a machine >>> that's had two patches applied to it (SA/EN as published by the >>> Security Team): >>> >>> bauer 11 % cat uname.c >>> #include >>> #include >>> #include >>> >>> int >>> main(int argc, char *argv[]) >>> { >>> =A0 =A0 =A0 =A0struct utsname un; >>> >>> =A0 =A0 =A0 =A0if (uname(&un)) { >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0perror("uname"); >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0exit (1); >>> =A0 =A0 =A0 =A0} >>> =A0 =A0 =A0 =A0printf("sysname: %s\n", un.sysname); >>> =A0 =A0 =A0 =A0printf("nodename: %s\n", un.nodename); >>> =A0 =A0 =A0 =A0printf("release: %s\n", un.release); >>> =A0 =A0 =A0 =A0printf("version: %s\n", un.version); >>> =A0 =A0 =A0 =A0printf("machine: %s\n", un.machine); >>> } >>> bauer 12 % ./uname >>> sysname: FreeBSD >>> nodename: bauer.cse.buffalo.edu >>> release: 8.0-RELEASE-p2 >>> version: FreeBSD 8.0-RELEASE-p2 #0: Fri Mar 26 16:58:16 EDT 2010 >>> root@bauer.cse.buffalo.edu:/usr/obj/usr/src/sys/FIREWALL >>> machine: amd64 >>> bauer 13 % >>> >>> So unless I'm mis-reading your patch it would be looking for >>> packages in >>> >>> =A0/ftp/pub/FreeBSD/ports/amd64/packages-8.0-release-p2 >>> >>> which doesn't exist. >>> >>> That problem isn't too hard to solve but the other problem is. >>> There are times during release cycles that branches wind up >>> with even weirder names than just tacking -p on to >>> the end of the name. =A0For example during the 7.3 release cycle >>> the stable/7 branch was named 7.3-PRERELEASE during the entire >>> cycle. =A0Once it got created the releng/7.3 branch was named >>> 7.3-RC1, and progressed to 7.3-RC2. =A0And take a look at what >>> a system installed from one of the Monthly Snapshots gives for >>> uname output, I don't have one handy at the moment but if I >>> recall correctly it has the snapshot's name embedded in the >>> uname output. =A0The mechanism that does that is what I use to >>> name the BETA releases as well, I never actually commit the >>> BETA1, BETA2, etc. names to a stable branch because it tends >>> to freak out people using those branches (we wind up getting >>> mail saying "Hey, RELENG_7 is a stable branch! =A0Why does >>> a machine updated today on RELENG_7 say it's *BETA1*???") >>> during release cycles; the PRERELEASE thing is an attempt >>> to avoid that...). =A0If you do a release build specifying >>> BUILDNAME on the command line it will use that as what gets >>> put into sys/conf/newvers.sh as the $RELEASE. =A0And that's >>> the source of what uname gives as the release field. >> >> =A0 =A0Ouch. You pointed out a flaw in my assumptions that would >> definitely invalidate this proposed change. Now I'm teetering between >> whether or not it's wise to actually make this change. >> >> =A0 =A0Here are some questions though: >> >> 1. What happens if compat libraries are used with a specifically built >> copy of pkg_install? Game over, right -- because the __FreeBSD_version >> is encoded in the binary? >> 2. Should prereleases really be allowed to use release-based packages? >> Probably not right -- generally the functionality is fixed in each >> release, but it can change dramatically before the official release is >> made, correct (take the 7.0-RELEASE for example...)? >> 3. What also happens if FreeBSD developer goes and messes up a package >> before the release 7.2-RC2, but it was working in 7.2-RC1 -- the >> individual will be toast right because they'll `automatically upgrade' >> to the latest version and can't go back to the earlier version without >> grabbing the CD, correct? One other thing that would be nice is that folks can now split up releases into multiple trains, say if they built package set A with a certain set of options, and package set B with a separate set of options. If I were to install package set B on a series of mips boards with Cavium support, they might not have the same set of requirements, or CFLAGS enabled that a mips port to qemu might have. Maybe a getenv check to $UNAME_r should be added s.t. this falls back to old behavior in the event of there not being a release defined in the environment? I'm still not sold on the fact that that would be the best solution because some degree of parsing needs to occur -- perhaps some sscanf fun with (struct utsname).release would be the the best way to go (scan for integers and the first period -- set the release accordingly) if $UNAME_r is undefined? Thanks, -Garrett State-Changed-From-To: open->repocopy State-Changed-By: flz State-Changed-When: Thu Apr 1 17:18:21 UTC 2010 State-Changed-Why: http://www.freebsd.org/cgi/query-pr.cgi?pr=145100 Responsible-Changed-From-To: freebsd-bugs->portmgr Responsible-Changed-By: flz Responsible-Changed-When: Thu Apr 1 17:19:16 UTC 2010 Responsible-Changed-Why: pkg_install is maintained by portmgr. http://www.freebsd.org/cgi/query-pr.cgi?pr=145100 State-Changed-From-To: repocopy->open State-Changed-By: marcus State-Changed-When: Mon Apr 5 02:49:57 UTC 2010 State-Changed-Why: I don't see what needs to be copied here. Responsible-Changed-From-To: portmgr->flz Responsible-Changed-By: marcus Responsible-Changed-When: Mon Apr 5 02:49:57 UTC 2010 Responsible-Changed-Why: I don't see what needs to be copied here. http://www.freebsd.org/cgi/query-pr.cgi?pr=145100 Responsible-Changed-From-To: flz->portmgr Responsible-Changed-By: flz Responsible-Changed-When: Mon Apr 12 11:09:49 UTC 2010 Responsible-Changed-Why: portmgr looks after pkg_install. http://www.freebsd.org/cgi/query-pr.cgi?pr=145100 State-Changed-From-To: open->suspended State-Changed-By: flz State-Changed-When: Sat Apr 24 11:36:58 UTC 2010 State-Changed-Why: Suspend for now, we'll discuss this in BSDCan. http://www.freebsd.org/cgi/query-pr.cgi?pr=145100 State-Changed-From-To: suspended->open State-Changed-By: eadler State-Changed-When: Mon Jan 2 15:27:22 UTC 2012 State-Changed-Why: bsdcan passed, any thoughts? http://www.freebsd.org/cgi/query-pr.cgi?pr=145100 >Unformatted: