Tags:
create new tag
, view all tags

Feature Proposals » Add CGI.pm to TWiki core distribution

Summary

Current State: Developer: Reason: Date: Concerns By: Bug Tracking: Proposed For:
ImplementedAsExtension HaraldJoerg AcceptedBy7DayFeedbackPeriod 2015-03-19   Bugs:Item7620 KampalaRelease

Edit Form

TopicSummary:
CurrentState:
CommittedDeveloper:
ReasonForDecision:
DateOfCommitment:   Format: YYYY-MM-DD
ConcernRaisedBy:
BugTracking:
OutstandingIssues:
RelatedTopics:
InterestedParties:
ProposedFor:
TWikiContributors:
 

Motivation

Lee Johnson, current maintainer of CGI.pm, has informed us that there will be changes in that module which will affect TWiki.

We have learned that TWiki is being used with various versions of Perl, and with various versions of modules, maintained with various upgrade policies. It will be an increasing pain to write TWiki code running with different versions of CGI.pm.

Description and Documentation

TWiki depends heavily on CGI.pm, on its good as well as on its not-so-good parts. If CGI.pm is made part of TWiki, then we only need to write and test for one version of the module, and we can get rid of the not-so-good parts within the regular TWiki release cycle, by removing obsolescent code and then upgrading the module within TWiki as we see fit.

Examples

If you look at configure, you see the comment "Versions 2.89 and 3.37 must be avoided". This is not the first time that TWiki's dependency on CGI.pm creates issues.

Impact

TWiki code is more streamlined and easier to test: There's only one version of CGI.pm to test against. The removal of CGI.pm from Perl's core will probably increase the variance of versions we'll encounter in the field.

WhatDoesItAffect: Install, Refactoring

Implementation

One tricky part I have spotted so far is that TWiki's own libraries and its CPAN copies are searched only after the system's directories have been searched. We need to override the system's or distribution's module if it does not fit to the current TWiki version.

-- Contributors: Harald Jörg - 2015-02-24

Discussion

Yes, the API of CGI is has changed with versions, and Lee intends to make even more changes based on his 5 minute talk he gave last year: https://www.youtube.com/watch?v=BBYag43ojjM. Not good for TWiki, which is installed in so many different environments.

It makes sense to ship TWiki with the CGI module to avoid the moving target. Are there any dependencies that could bite in the future?

Would it make sense to rename the CGI module to something like TWiki::CGI and have it part of the core?

-- Peter Thoeny - 2015-02-24

I just looked at CPAN:CGI v3.43, http://search.cpan.org/~lds/CGI.pm-3.43/CGI.pm, it has a big dependency list:

What is the best approach to handle this big dependency list?

-- Peter Thoeny - 2015-02-24

Apart from HTML::Entities those are all core perl modules, and half of them are pragmas. You only have to worry about the none-core perl modules.

-- Lee Jo - 2015-02-25

Copying a "verified" version of CGI as TWiki::CGI is straightforward and gets TWiki completely out of the way of CGI's development. It has its quirks, though: The CGI distribution contains half a dozen of modules in the CGI namespace, used also internally within CGI.pm. Mixing different versions sounds like a recipe for failure, so I recommend to copy and rename all these modules, and to patch all lines which use and require the modules internally. Gradually following changes in CGI by rebasing TWiki to new versions would, in addition to the changes in the TWiki codebase, require to repeat this procedure every time.

Should TWiki follow changes in CGI? Or should it just freeze CGI at some stage? Or should it aim at replacing CGI altogether, e.g. with one of the alternatives shown in CPAN:CGI::Alternatives? Personally, I think freezing is good enough, and it leaves both options to get rid of it in the future, and to cherry-pick changes in "original" CGI. In the last months we occasionally had support questions from people who installed CGI 4.05 or newer from github, inadvertedly breaking TWiki. Freezing our own copy of CGI will avoid similar problems, too.

As for the list of dependencies, I consider all of them harmless. The interfaces of HTML::Entities and HTML::Tagset are very narrow and, in my opinion, extremely unlikely to change. They should not be added to TWiki, though, because HTML::Entities uses XS parts (via HTML::Parser in the same distribution).

-- Harald Jörg - 2015-02-25

Harald, makes sense. Sound proposal to just rename/freeze CGI at some stage.

-- Peter Thoeny - 2015-02-25

Maintaining your own fork with a different namespace is probably the way to go, although it will be a little more involved than just patching the runtime and compile time imports - there are a lot of package variables and such in CGI.pm and all of these will need to be updated to. Fortunately it's a bit simpler now i've moved a lot of the runtime AUTOLOAD magic.

It may be easier to take the Bugzilla approach and subclass CGI and override any methods you need to, but that may not quite get you where you want to be.

If you do take the fork approach then look at doing this via github, it will then be trivial to send any patches upstream to CGI.pm as well as trivial to keep your fork up to date with upstream changes too.

-- Lee Jo - 2015-02-26

Oh - excellent catch with the package variables. I've overlooked these. When writing my comment, I had the crazy idea of just changing the path of the module, but not the package name, so we'd use TWiki::CGI and still end up with package CGI. However, this fails in crazy ways when some part of the code (maybe an unpublished plugin) does an unaltered use CGI: CGI.pm will be loaded again, overwriting package variables and subroutines, and getting noisy when redefining the "constant" XHTML_DTD.

I am not too confident with subclassing either, because we'd again stumble over of a great variety of CGI versions to subclass from.

After a few glimpses at the code and experiments, I suggest a different approach: We can just fork without changing path name nor package name and just place some stable version of CGI.pm in the same directory as TWiki.pm. The TWiki directories are prepended into @INC before the system paths, whereas TWiki's CPAN copies (which hold, for example, CGI::Session) are pushed to the end of @INC.

This would not require any changes in TWiki's codebase, and be robust against stray use CGI in the code, and robust against distribution/local installations of any version of CGI.pm. I could think of very exotic setups where it fails (mod_perl with another application in the same server, which picks the system's CGI.pm before TWiki loads its own).

-- Harald Jörg - 2015-02-28

Yes, there are plugin that use CGI. So Harald's suggestion is a pragmatic approach.

-- Peter Thoeny - 2015-03-01

Ok, I'll try to stay awake for KampalaReleaseMeeting2015x03x05 to introduce the topic into the process smile

As for the fine print: I don't know whether the following idea is covered by TWiki's release building process: Instead of simply dropping a CGI distribution to SVN trunk/core, we should add a trunk/CGIContrib and expand CGI there. The release build would then need to pick it from there, similar to the usual default plugins, into the distributed tar.gz file. As far as I understand, the selection of files to be shipped would be done according to trunk/CGIContrib/lib/TWiki/Contrib/CGIContrib/MANIFEST (to be created) and therefore not interfere with CGI's own MANIFEST, which ends up at trunk/CGIContrib/MANIFEST.

We would then just pseudo-install TWiki's local CGI when testing from SVN, with an easy option to fall back to the system's CGI for comparison.

-- Harald Jörg - 2015-03-01

thumbs up

-- Peter Thoeny - 2015-03-01

We discussed this at KampalaReleaseMeeting2015x03x05: Consensus is to implement as discussed as a new contrib, and to ship this contrib with the TWiki core distribution.

This proposal needs a committed date and developer. Harald, are you up for this?

-- Peter Thoeny - 2015-03-05

If you are going to take a fork i would suggest waiting for the 4.14 release of CGI.pm, which i should have out early April, and forking that as it will be a much cleaner version than the current code.

-- Lee Jo - 2015-03-06

Hmm, I think it is easier to use a current stable version. Most recent ones produce many "CGI::param called in list context" errors, which prevents the configure script from running (see for example Support.SID-01980). Question if to invest time in fixing the large TWiki code for the latest CGI changes, or to simply use the latest CGI version that works.

-- Peter Thoeny - 2015-03-06

Since I intend to pick the whole CGI distribution into the contrib, rebasing won't be difficult at all. I have to admit, however, that I don't plan to make changes within CGI, I'd rather clean up the uses of CGI within TWiki over time. So it is more a freeze than a fork, and for each new TWiki release we can decide whether rebasing to a current CGI is of any benefit to TWiki. I am not concerned at all about the "CGI::param called in list context": It is very easy to suppress the message, and I suggest to do this. The suppression is completely harmless even if some installation manages to use its own ancient version of CGI (it is just a package variable unused in old versions). I also suggest to enable the warning during tests.

And that's the only point which may need some work: Today's automatic unit tests can't catch the interactions between the CGI module and the web server too well.

I've added myself as a committed developer. I already run the code in my own development version (featuring CGI,pm 4.13 from CPAN) without any problems, so there's just the Contrib bookkeeping to be done.

-- Harald Jörg - 2015-02-28

Sounds like a plan. Thanks Harald!

Best to do in trunk and 6.0 branch so that this is released in next patch release.

-- Peter Thoeny - 2015-03-07

On its way: Bugs:Item7620.

There's a related issue which needs to be covered in this context: Bootstrapping. configure relies on finding a CGI.pm before TWiki's directories are added to @INC. As long as Perl versions still have CGI in their core, it is just sufficient to add $CGI::LIST_CONTEXT_WARN = 0; to the configuration code. We need some more intelligence in configure (which isn't a bad idea anyway) as soon as Perl versions without CGI in their core become prevalent.

So right now this is only a safeguard against people installing Lee's bleeding edge CGI versions to their Perl, which I consider to be good enough.

Some further investigation also revealed a probably uncritical issue with CGI's recently-acquired dependency HTML::Entities. While I still consider this dependency harmless with regard to API stability, it adds a requirement to the installation procedure: HTML::Entities uses XS, therefore a compiler is needed for installing it from CPAN (I already wrote that we can't ship it with TWiki for the very same reason).

I call this "probably uncritical" because

  • Linux/Unix folks usually have all things which are required to build HTML::Entities
  • For Windows, HTML::Parser (the distribution containing HTML::Entities) is available in Activestate Perl, and Strawberry users can build it like in Linux/Unix.
  • WysiwygPlugin (and various other plugins which I consider less important for the cause) already features a dependency on HTML::Entities, apparently without someone complaining so far.

-- Harald Jörg - 2015-03-07

I had a shot at CGI 4.14 today, and it breaks TWiki, again. There are a four places (configure, BuildContrib, and test fixture plugin) where TWiki code is using CGI with the :any tag, but this tag is no longer supported. Right now investigating whether the ability to invent arbitrary HTML elements is actually used by TWiki is not my top priority, though. TWiki 6.01 will not work with CGI 4.14, and the next release might as well have 4.13 bundled with it, and go for eliminating the CGI-based HTML creation functions completely in a minor release.

-- Harald Jörg - 2015-04-13

Support questions related to CGI.pm keep coming in, e.g. Support.SID-02103. All of them come from RedHat systems, which apparently doesn't ship CGI.pm as part of their Perl distribution, thus forcing users to install from CPAN. I wasn't aware of this situation and thought we only needed to care for Perl 5.22 and above, where CGI was removed from the Perl core. Therefore, the CgiContrib in SVN is based on CGI version 4.13, and will only work in KampalaRelease because a few lines in TWiki (or maybe just one line, if I recall correctly) needed to be changed for 4.13 to work.

Would it be preferable to downgrade CgiContrib to CGI version 4.03, which will work "out of the box" with old versions of TWiki, and upload that to twiki.org? In that case, installing CgiContrib would just be sufficient to get rid of the problems. For Kampala onwards, where CgiContrib will be part of the TWiki distributions, we still could occasionally rebase to current CGI if appropriate.

-- Harald Jörg - 2015-09-19

I've now uploaded CgiContrib as a solution for people using TWiki 6.0.1 who don't want to install "outdated" CPAN modules. Installing this contrib should fix CGI problems in current versions while allowing a seamless migration to KampalaRelease. Tests with a pristine Fedora server which doesn't have CGI installed are yet on my todo list.

-- Harald Jörg - 2015-10-28

Thank you Harald!

-- Peter Thoeny - 2015-10-28

I enhanced the configure script so that CGI and CGI::Carp are taken from this extension, not the system libraries.

-- Peter Thoeny - 2015-11-28

Edit | Attach | Watch | Print version | History: r23 < r22 < r21 < r20 < r19 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r23 - 2015-11-28 - PeterThoeny
 
  • Learn about TWiki  
  • Download TWiki
This site is powered by the TWiki collaboration platform Powered by Perl Hosted by OICcam.com Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.
Copyright © 1999-2017 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.