Add version info to modules page

haven't looked at the code, but a huge +1 to displaying this info.

one of my thoughts from the new release system would be that the new packaging script could automatically insert the right version value into the .info file(s) in a given release when it's being created. that way, authors never have to worry about this crap. once you tag a given release, the version info is set in stone, and will always match between the packaged release, the release node, etc.

the only downside with this approach is that folks checking out modules directly from cvs won't have this goodness. :(

the only downside with this approach is that folks checking out modules directly from cvs won't have this goodness. :(

If we standardize on using something like this in the info files:

version: $Name$

Then this value would be available to people who get the tarball AND to those who pull from CVS directly.

There's nothing that stops module maintainers from adding .info files to the 4.7 versions of projects and putting this information in there as well.

The drawback I see to this approach is that it being a manual action by module developers means that some of them will forget to do it. Not a huge deal, but it's less reliable than having the packaging script do this work.

Also, a small usability improvement would be to link that version number to the release node on Drupal.org. If we use the $Name$ as the version then we need to have something that goes like: drupal.org/project/pid/cvstag and forwards it to the release node for that tag.

Now that the new release system is in, we can actually do this. I guess Derek's talked w/ Dries and Dries has given this a green light. Woohoo!

Here is a patch which adds:

version = "$Name$"

to all core files in addition to the changes in merlin's first patch.

Note that at the moment this patch doesn't look like much; it just shows $Name$ as the version of all modules.

But! When these .info files are committed to core, this expands out into the branch of the thing that was committed.

This, coupled with some prettifier stuff in cvslog module will convert things like DRUPAL-5--1-2 to "5.x-1.2" in this listing for tarballs.

However, this is the first step.

we've had some new debates about this thread, and i wanted to condense my thoughts in 1 place. here's how it should work:

• core just reads whatever's in the .info file in the 'version' field and displays it directly. that seems to be exactly what merlin's patch does (though i haven't played with it yet). no fancy formatting or attempting to parse $Name$ tags, or we're doomed:
  • we'd totally screw over anyone running modules not on d.o infra, not in CVS, not using our tagging conventions, etc.
  • we'd never be able to change anything about our tagging/branching again (not that we'd need to, but still) ;)
  • (again, not that we'd want to ;), but...) we'd never be able to move to SVN or something else (unless we're sure it handles $Name$ exactly the same way), etc.
  • this would bloat core for everyone in the name of more readable version strings for the small (tiny?) minority who use CVS as a distribution tool, not as a development tool.

• the packaging script for the new release system will do the nice pretty-printing, and will always insert (or replace) any version line in the .info file for something it's packaging with version = 5.x-1.3 for a real release, or version = 5.x-1.x-dev for a snapshot. this will exactly match the name of the tarball, the thing in the issue queue, the handbook, etc.

  these packaging changes would be trivial. the script already (duh) has the right version string to use. so, after it checks out from the right tag or branch, it just has to look for any .info files (there might be more than one in a project with N modules under it, like, oh, project and project/release). ;) all .info files in the same project (and therefore in the same CVS tag and same tarball) get the same version string. once we find them, it's a trivial preg_replace() on each one before we tar it all up and continue on our merry way.

  oh, and there's no reason it couldn't always add at least 1 project_short_name.info file with the strings it uses in the tarball name, for example:

  name = project
  version = 4.7.x-1.0
  

  even for 4.7.x or 4.6.x packages it creates...

• module maintainers should be encouraged/forced to put version = $Name$ into their .info files. we could write a script to blast through all existing .info files, we could make the xcvs-commitinfo.php script refuse to allow users to commit a .info file that didn't have this line, etc. ;) this only helps the advanced users who run their sites directly out of cvs, so they at least get some kind of data (even if it's ugly).
• since the module page is just a big rendered form (at least looking at merlin's patch -- i've been out of touch with 5.x core development...), webchick's regexp-based $Name$ parser could just be a form_alter()ing contrib and still replace the info before core puts it on the screen.

best of all, Dries said this can happen before 5.0-RC (since it's such a small change, but it'll be oh-so-wonderful an improvement to make the new release system all the more sweet. ;)

in the future (6.x), we would consider how version-based dependency checking would work (e.g. project_issue 4.7.x-2.* only works with project 4.7.x-2.* -- it'd be nice to encode that in the .info file and enforce it in core). the problem is that version strings in the .info file aren't always exactly the same format (e.g. "5.x-1.3" vs. "$Name: DRUPAL-5--1-3 $" vs. "Lullabot_V3.2" vs. "CiviCRM-2.3" vs...). this seems tricky, but no reason to complicate the 5.x solution outlined above.

so, is merlin's patch ready? myself or kjartan could easily do the packaging script changes, though the code (modules/project/release/package-release-nodes.php on the DRUPAL-4-7--2 branch) is pretty darn obvious and (if i may say so) well commented and laid out, so i'm sure anyone else could do it, too.

thanks!
-derek

[bumping to critical since we must move fast to get this RTBC and committed before the RC].

It's rock'n'roll time I think. Derek said he will work on the packaging script...

not quite so fast... do we really want:
version = "$Name$"
shouldn't it be:
version = $Name$
???
we don't use " on anything else in there...

ok, in IRC merlin and neclimdul clued me in that in ini-formatted files, you have to quote non-alphanumeric chars:
http://us2.php.net/manual/en/function.parse-ini-file.php
those $s in $Name$ could be trouble. so, carry on, then... ;)

meanwhile, i'm working on a patch for the packaging script. i'll reply here when it's ready.

see http://drupal.org/node/97286 for the packaging script changes for this. needs review...
thanks,
-derek

This is an fine proposal it seems - we definately should show the version info. If we put the file size or last modified into the .info, we could probably also show when a file has been customized (an easy way to highlight problem areas when upgrading).

I had proposed a totally different way to collect this info where each file provides its own version info. At the top of each file, we change the $Id$ line so that it reads:

$GLOBALS[dirname(__FILE__)] = $Id$

this is nice because packaging script is not involved.

in the real implementation, we would use a relative path to key the version info, not absolute like above. for example:

function GetRelativePath($path) {
   $npath = str_replace('\\', '/', $path);
   return str_replace($_ENV('DOCUMENT_ROOT'), '', $npath);
}


GetRelativePath(dirname(__FILE__));

The problem is, $Id$ spits out something really silly like 1.353, which means absolutely nothing.

$Name$ will spit out something like DRUPAL-5--1-2 which is a bit awkward but at least tells you where to look.

dww's packaging script would then parse that and make it "5.x-1.2" which is quite obvious.

dww's packaging script would then parse that and make it "5.x-1.2" which is quite obvious.

technically, no. ;) my packaging script just does what it's told from the release node -- whatever you put for Version string in there is what gets used for a) the tarball name, b) the issue queue, and now c) the value of the version line in any .info files in the directory tree of that release.

however, the only way to specify the Version string is from selecting the CVS Identifier on page 1 of the release form, or, if you happen to have Administer projects permissions and go in to edit an existing release node. so yeah, effectively, my script (really cvs.module) parses the name of the tag, and makes the nice human-readable version string.

the main point is that the packaging script already knows the right value, and by brute force will clobber whatever info happens to be in the .info files in CVS, and it adds a version = ... line to any .info files that don't specify it at all. this defends us against:

• maintainers who totally forget
• maintainers who foolishly try to put their own version strings in there (utter receipe for stale info or confusion)
• any other potentially faulty, unparseable, confusing data in there

if maintainers really want to customize the version string for some reason, they can always put some additional junk at the end of their CVS tag for that release, which gets turned into the "extra" field of their version string, and propagated through the system. this was originially intended just for "beta2", "rc1", etc, etc. but, in theory, it could be used for anything (so long as it's [0-9a-zZ-z_-]). ;) see http://drupal.org/handbook/cvs/branches-and-tags/contributions for the canonical reference on allowed branch/tag names in contrib.

i know webchick is hoping to remove this add-if-not-there behavior to force contrib maintainers to put in version = "$Name$" so that life is better for those running sites directly out of CVS, but i'm not convinced this is an important enough reason to generate a bunch of work (and new releases) just so that maintainers add a version line. but, i'm open to debate on this (it's only about 5 lines worth of code to rip out of a single function in the packaging script if we want to disable it)

make sense?

Heyyyy!

Something merlin clued me into is that parse_ini_file will automagically make VERSION translate into the VERSION constant used by system.module.

So here is an updated patch that, instead of doing version = "$Name$" now does version = VERSION, making this actually useful right now in HEAD. ;D

Thanks to Morbus for this useful one-liner:

perl -pi.old -e 's/"\$Name\$"/VERSION/g' modules/*/*.info

Screenshot.

i'm a bit confused. why would we want to put the exact same number as the version of all our modules? I refer to the last patch which does version = VERSION in each file.

also, the point of the packaging script here isn't really to translate a CVS tag into a prettier format. that could be done in drupal code too. the point (as i understand it) is to grab the version of the .module or .theme file and put that info into a *different file*, namely the .info file.

of course, in my scheme it could very well be $GLOBALS[dirname(__FILE__)] = $Name$ so we don't have to parse the Id string but I see no point - that ID string built to be parsed and has lots of useful info in it.

i should elaborate - the version = VERSION will show the same number for all core and contrib modules which is not what we want.

Yes, what merlinofchaos said about VERSION. Contrib module authors should _not_ do this, but it works for contrib modules.

@moshe, I'm still not totally clear on how your version would work.... Could you do up a patch? It sounds like @ admin/modules it would show stuff like:

aggregator - 1.312
block - 1.234
...

How does this help people to understand which version of a module they're running?

Contrib module authors should _not_ do this, but it works for contrib modules.

Sigh. I really should not respond to issues while I'm doing 10 other things. :P

but it works for core modules, that should read. ;)

also, the point of the packaging script here isn't really to translate a CVS tag into a prettier format. that could be done in drupal code too. the point (as i understand it) is to grab the version of the .module or .theme file and put that info into a *different file*, namely the .info file.

you're confusing the revision of a specific file with the version of a release of the whole package. all files in a package have the same version (the maintainer-defined identifier for this particular release). every file in a package has a different revision (the CVS-defined unique identifier for that particular revision of that particular file). this is effectively the difference between $Name$ (the tag, package-wide) vs. $Id$ (the revision, specific to each file).

it makes no sense for the modules page to show you the per-file revision info for every file in a given module. that's information overload and would be totally useless info for 99% of users that aren't developers. if you really want/need to know the $Id$ (revision) info for some reason, you're clearly smart enough to look in the filesystem. if you just want to see "what versions of each module am i running?" as a mere-mortal drupal admin, you want to see stuff like "5.0-beta1" for core modules, and "5.x-1.0" for OG, and "5.x-1.2" for og_views, and ...

the packaging script makes this so, for all packages, core and contrib, regardless of what (if anything) is in the .info files in CVS.

the remaining debate is simply to make life somewhat better for people who choose to deploy drupal sites directly from CVS, instead of downloading the official packages. for them, core can specify version = VERSION to automatically get whatever system.module defines for VERSION (assuming we remember to keep that accurate *grin*). contrib can specify version = $Name$ so that at least you get something like "$Name: DRUPAL-5 $" (if you check-out from the end of the DRUPAL-5 branch) or "$Name: DRUPAL-5--1-2 $" (if you checkout a module from a specific release tag).

someone could write a contrib module (run_sites_from_CVS.module) to translate these ugly "$Name: ... $" strings into a nicer format (e.g. "5.x-dev" or "5.x-1.2" in the 2 above examples), but this logic MUST NOT be in core for all the reasons i explained in #5 above. such a contrib is *only* a convineince for the advanced admins that deploy sites directly from CVS.

all clear? ;)

moshe: sorry, maybe i just didn't fully comprehend your proposal, and webchick's question helped clarify.
maybe in the special-case of core modules, it'd be desireable for the "version string" to be something like:

book: 5.0-beta1 (rev  1.394)
comment: 5.0-beta1 (rev 1.494)
contact: 5.0-beta1 (rev 1.68)
...

that could be interesting for us, but i fear it would confuse mere-mortal site admins... also, it's misleading for things like node.module, which also have a content_types.inc file. that's probably why you're advocating a huge array of the revision of each file (.install, too, etc), but i still think that's completely information overload for the basic modules page UI. how would we display this data?

also, keep in mind that people *can* run different versions of core modules on the same site, e.g.:

book: 5.x-dev (rev  1.395)
comment: 5.0-beta1 (rev 1.494)
contact: 5.0-beta1 (rev 1.68)
...

(if they used the book directory from the latest 5.x-dev tarball) so, even without the "(rev xxx)" part, it's not completely insane to display the version for each core module, since they're not necessarily *always* the same, and if not, it'd be useful to see that.

anyway, moshe, is that what you have in mind? if so, it'd certainly be easy to do in the packaging script. again, the straight-from-cvs crowd would complicate things. but, those are exactly the people that already know how to use "cvs status" or look in the $Id$ if they need to know specific revisions.

or, their "make_versions_pretty_from_CVS.module" could also parse the first few lines of each enabled .module file looking for the $Id$ tag and add the "(rev xxx)" to the displayed version info if they really wanted... but, again, this has no buisness being in core.

I didn't really know how i wanted to display all that info (if ever). Anyway, you guys have a better handle on this than i. please proceed however you see fit.

This is taking up a lot of real estate. What about putting the version after the module name or after the description, maybe next to dependency information.

Why would this be needed for core modules? Modules are grouped so we can be smart about when it shows.

It is a little bit weird to add 'version = VERSION' to every .info file. In essence, it really means 'this module is a core module'. There might be more elegant ways to detect core modules.

I understand the reason why the 'version = VERSION' trick works out. It's clever but too obscure for most people to grok. IMO, we want these files to be readable and understandable by non-developers. It's not the best place for sofisticated tricks like this.

Dries asked the fair question, "Why version = VERSION on every core module? Why not leave it off, and let the absense of a version string be the designation of a core module?"

Here's why.

Derek has a nice patch so that the packaging scripts can automatically stick in the proper version string for tarballs. So if I have a module that's tagged DRUPAL-5--1-3, it will generate a "version = 5.x-1.3" string and stick it in my .info file if it doesn't exist (and replace the existing one if it does).

However, this will NOT work for people who checkout and maintain core and modules through CVS. If a module maintainer forgets to include a version = "$Name$" in their .info file, these versions will come up totally blank for CVS site maintainers.

Therefore, a blank version string isn't a good indicator of a core module, because a string can be blank due to a maintainer making a mistake and forgetting a version = "$Name$" line.

And therefore, core modules need to have version info. version = VERSION provides a nice short-cut to automatically have them "inherit" from whatever version core is, while at the same time allowing the flexibility of people to override this manually if they end up patching it in some way, as a signal for when they're upgrading.

Hopefully that makes sense, and helps illustrate why we went this way.

I'm not sure it makes sense. Even if we need a version string in the info-files, why don't we have a unified solution for core and contrib?

Here is a patch for a UI refinement. The version number column is merged with the name column. The version number only shows when it is the most relevant, when it is different from Drupal core's version.

I've put not thought into whether 'version = VERSION' is good or not; that debate doesn't look settled.

Here is a screenshot of what the page would be like if another version of aggregator somehow got in.

@drumm: this rocks! mostly i love it, and i'm totally fine with not including the version next to each core module if they're all the same. your patch is the best of both worlds: save the space for other things, but use it if there's something interesting to know.

my *only* complaints are:

1. we should at least display the current version of core *somewhere* on this page. i know it exists on the "status report", but that's a fairly counter-intuitive place to go look (i'm supposed to find my version of core under the heading "Logs"?). especially since everything else tells you the version on this page -- we should keep all the version info in 1 place, IMHO. i'm not sure exactly how it should look and where it should live, but i'm sure it should be there somewhere... i'm too tired now to have any bright ideas, but i'll post something tomorrow if no one else has a solution yet.
2. sadly, VERSION depends on the humans keeping it accurate. we're already not doing this, since beta1 went out with VERSION still set to "5.0 dev" (not "5.0-beta1" as it should have been) :( as soon as i commit the patch to the packaging script, it'll be impossible for the data in the .info files to disagree with the version, but it will be possible that VERSION doesn't match the version of the release. not sure what to do about this, but i wanted to raise it here.

@dries: I'm not sure it makes sense. Even if we need a version string in the info-files, why don't we have a unified solution for core and contrib?

we do have a unified solution: http://drupal.org/node/97286
the packaging script will never lie and will never forget.

however, all you pesky deploy-straight-from-CVS people continue to make my life difficult. ;) (dirty secret: i'm one of them, too *grin*). for the deploy-from-CVSers, all the goodness of the packaging script isn't available. so, what gets left in the .info files in this case is totally up to the maintainer of the module. in the case of contrib, depending on people to remember to update their own "EVENTS_VERSION" constant (or something) is asking for trouble and stale info. so, the current plan is we'd recommend using $Name$ (which CVS automatically expands to the name of the tag or branch you're checking out from. however, if someone wanted to manually keep their .info files updated with version = 5.x-1.3 before they make their release tags, that'd be fine, too.

in the case of core, since we're already going through the trouble to maintain the VERSION constant, we can use this sneaky trick so we a) don't have to edit *all* the .info files each time and b) we can provide even more data than just "DRUPAL-5" branch (which is all $Name$ would tell us most of the time).

the whole question of what gets checked into CVS for the version fields in the .info files only matters for people who deploy from CVS. i think this is going to become a smaller and smaller population in the drupal community, as people build better deployment tools on top of the new release system foundation. personally, i don't really care what's committed to CVS as the "version" line in the .info files for core... i just want the UI on the modules page to go in, and i want to commit that change to the packaging script. everything else is only a convinience for the advanced users deploying via CVS.

drumm: your UI looks less intuitive to me. Where is the version number? There is only one, right now ... looks like a broken UI to me.

Putting the version number after a module or product name is common practice. If a series of digits with dots and other characters is seen behind a module name there is no mistaking that it is a version number. Taking them out of their own column is okay and simplifies the screen by not requiring another column heading for data that doesn't need a label.

Version numbers are important for contributed modules and especially important for spotting inconsistencies in hat you are running. For core modules, they will all have the same version. There is no need to repeat that 29 times. With the current patch, I expect all contributed module version numbers will show since '5.0' != '5.x-1.0'. The relevant information will be shown where it is needed, for contributions and core modules someone wants to obviously label as hacked.

I agree this needs to get in, but I don't want to see the table get unwieldy.

i think 1 line at the top like:

All core modules are version 5.0-beta2 unless otherwise noted.

would solve the first problem i mention in #31, and would help clarify what's going on.

I really see no problem with repeating the version number. For me, webchick's screenshot is the aesthetically more pleasing one. And it's also easier to grok. I think it is unrealistic to assume that people will think: "the missing version numbers probably indicate that they are all identical". This might be personal taste.

Yes, this is likely a matter of taste. Perhaps we show the version matches in light grey but any deviations in black.

Here is a new patch:
- Version information is shown only for contributed modules.
- At the top of the two core fieldsets, "Drupal version 5.0 dev"

It would help to have screenshots of the different approaches. From my testing, I still prefer the first approach but it's been a couple days since I installed that version.

Do we have any contrib modules with version information? Will that be automated somehow?

If we do, then screenshots are easy and will have real data.

FYI: http://drupal.org/node/97286 is now committed and installed on d.o, so the next packaging run will automatically add accurate, human-readable version lines to any .info files in both core and contrib. see that issue for details if you care. ;)

-1 on patch #38 for a few reasons:

1. it's no longer possible to display if there's any "version skew" in your core modules folder
2. it trusts the VERSION define instead of what's in a .info file (e.g. system.info) as the default version for core. this is bad as i explained in #32.
3. even in contrib, the version string looks a little out of place, like it's an unlabled column (since the name is bold and the version is not).

i think i'm now leaning for webchick/merlin's simple approach: always display the version in its own column, perhaps w/ moshe's friendly amendment to use grey for all the core modules that match (e.g. whatever is in system.info), and black for any that don't match.

I think dww raises a valid point -- we should be able to spot conflicting modules. That is one of the primary goals of these version numbers. The other primary goal would be to figure out what version of a particular module you need. That seems to be the two goals we need to optimize for.

just to be clear, i'm now in favor of webchick's patch from #13. the UI is consistent. the code is simple, since there's no special-case to worry about for core vs. contrib. it allows you to see version skew across your core modules. it always uses the data from the .info file, instead of relying on VERSION (which is good, since the .info file is guaranteed to be the most accurate value we have).

drumm: i definitely respect your effort to avoid printing the same data a lot, but i think the results of that end up removing important functionality or making the UI less intuative in other ways. so, my vote would be for patch #13 and moving on to other things.

we'll need to revisit the module page UI in 6.x for version-specific dependency stuff, anyway. ;)

thanks,
-derek

+1 on webchick's #13 patch. As dww said, it's simple and straightforward. If your files inside a particular versioned package (or in core) are out of sync with each other, you need to reinstall the module or reinstall core. The version number stored in these .info files is the user-readable version number for a given project. That makes sense. Things like CVS revision ids don't really belong there, IMO.

My 2c on #13. If you have a bunch of same numbers, that's not a problem IMO. Simple is beautiful.

Also, we can add to security patches a special version for those files that are only security patched -- they are not 5.1 but not 5.0 either. 5.0-security1 maybe... so core will not always be uniform.

Committed to CVS HEAD. Thanks folks!

thank you!

minor nit: you accidentally committed some changes to sites/default/settings.php with this commit. attached patch restores it.

Committed to CVS HEAD. Thanks.