Packaging Open Journal System for Debian

[jerico]

...and another question...

The compatibility layer uses two kinds of classes / patterns:
1) proxy interceptors (for the database-specific datadict and driver classes that we can dirctly subclass, e.g. AdodbMysqlCompat)
2) delegate interceptors (for the base-classes that I cannot inherit from directly, e.g. AdodbConnectionCompatDelegate), think some kind of very basic AOP to avoid code duplication when "patching" base classes

Now I am unsure about how to name these classes correctly. For the moment I am calling the proxy interceptors ...Compat and the delegate interceptors ...DelegateCompat. Maybe this is not ideal. What's your opinion of this?

[jerico]

Hi Alec,

I've looked into #4357, #4359-#4367 and #4373 during the weekend. You may read the comments and attachments in #4357 first for general remarks and a graphical overview of all the patches I created. It would be great if you could comment during the week so that I could go on next week end.

I could not yet look into #4368-#4371. I'll do that later when I've time again.

I could not yet test all the patches. As I didn't find unit tests I'll have to do full regression tests manually. I'll probably write unit tests against the components I created to stabilize them for later maintenance. I admin that I am quite addicted to automized tests. I can't really imagine working without them any more.

IMO unit tests become even more important once you'll update 3rd-party libraries more regularly. If you have tests for all of the application's use-cases against the library you can be (quite) sure that 3rd-party library upgrades won't break your application, even if you have some rather exotic use-cases.

Florian

[asmecher]

Hi Florian,

Everything looks fine to me except for #4363 -- I've never been particularly happy with that fix. I'll look into improving it first so that it doesn't generate so much garbage, then we can see about getting some of the code duplication under control.

I take it you expect bug #4364 can be fixed upstream? It seems to me that many of the fixes are suitable for upstream adoption, but unfortunately ADODB hasn't been especially responsive.

I think regression testing for this is absolutely invaluable, particularly since many of the modified ADODB features are mostly used for upgrades and will therefore not be fully tested very thoroughly in the course of regular use.

Regards,
Alec Smecher
Public Knowledge Project Team

[asmecher]

Hi Florian,

FYI, it's easier to get details on the fixes by looking in the Bugzilla records for the particular commits in question. See, for example, http://pkp.sfu.ca/cvs/cvsweb.cgi/ojs2/lib/adodb/datadict/Attic/datadict-postgres.inc.php; it links to bugs #3677, #2343, #1864, #1863, etc. to provide more information on the particular bug each change was addressing.

Regards,
Alec Smecher
Public Knowledge Project Team

[jerico]

I take it you expect bug #4364 can be fixed upstream? It seems to me that many of the fixes are suitable for upstream adoption, but unfortunately ADODB hasn't been especially responsive.

I believe that most of the changes that have been applied to ADOdb by the OJS project should really be pushed upstream, not only #4364. The compatibility layer is only meant as a practical workaround as long as we cannot push all fixes upstream.

I only gave #4363 and #4364 a higher priority because they cannot be easily worked around through the compatibility layer. As long as these fixes have not been integrated upstream (or we have come up with another workaround), we have no chance of using the original ADOdb and therefore won't get OJS into Debian. This means that these bugs currently are show stoppers for Debian packaging while the other bugs are not.

Everything looks fine to me except for #4363 -- I've never been particularly happy with that fix. I'll look into improving it first so that it doesn't generate so much garbage, then we can see about getting some of the code duplication under control.

Can you please post #4363 and #4364 upstream and link to the upstream bug reports in Bugzilla so that I can follow up on these more closely? As soon as I got the links I'll post the same bugs against the ADOdb Debian package.

I think regression testing for this is absolutely invaluable, particularly since many of the modified ADODB features are mostly used for upgrades and will therefore not be fully tested very thoroughly in the course of regular use.

How are you currently doing regression testing? I assume now that you are not using unit tests for this matter. If I introduce unit tests for my changes, will you be able to maintain them and integrate them in the release process?

[asmecher]

Hi Florian,

Proper regression testing has been a burr under our collective saddle for a while; we have a fledgling structure in place for JUnit testing but we've been too busy to write proper tests for it. We've been handling testing manually with each release, focusing on the areas of the system that have been changed, occasionally making use of external testers when they've been available and particularly for larger releases. Unfortunately I don't have a solution for it currently, though we'll have to reopen the issue for more consideration as we get closer to the release of OJS 2.3, since there's such a large amount of new and changed code.

Bug #4364 has now been closed -- it turns out that a fix for this bug was already committed upstream to ADODB, making this fix actually break the code in question (which does not appear to be used by OJS anyway). I've backed the modification out of CVS.

I'll have a look at bug #4363 shortly and may need to submit a fix upstream to ADODB.

Regards,
Alec Smecher
Public Knowledge Project Team

[jerico]

Hi Alec,

I've received all your messages here and in BugZilla. Thank you very much! During the week I had a lot of work and this week-end I'll be traveling. But next week-end I'll be back and then I'll respond to all you have written.

Florian

[jerico]

we have a fledgling structure in place for JUnit testing

Do you mean PHPUnit? I think JUnit is for Java only...

I'll do my tests in PHPUnit.

we'll have to reopen the issue for more consideration as we get closer to the release of OJS 2.3

If you'd like me to share some experience with automated unit and frontend testing I'll be happy to do so. I've been working quite extensively with PHPUnit (+XDebug coverage measurement) and Selenium (+ its FF add-on). Even if you want to avoid a big upfront investment in covering everything you've done in the past it's still a good idea to stabilize with tests what you touch from now on. So over time you get at least a reasonable test coverage for those parts of the system that change often.

That's what I do for the compatibility layer:
1) I write unit tests against the original code. I make sure that I get 100% test coverage for the class methods I am about to change.
2) I make my changes
3) I run the unit tests after the changes to make sure that the compatibility layer behaves as the original code did

This is a lot safer and easier than doing manual regression testing.

[jerico]

A public subversion repository with the current state of affairs of the Debian packaging effort is now available. You can browse the code at [1] (web interface) or get it from [2] (anonymous svn).

Help and collaboration is welcome. Those willing to contribute should read [3]. If you don't have an Alioth account yet then register at [4]. Then go to [5] and request to join the collab-maint project. This will give you write access to the ojs subversion repository.

You'll have to generate and upload an RSA key to Alioth and acquaint yourself with the usage of svn-buildpackage. Write access to the repository is under [6].

Co-ordination of the packaging effort is in this forum topic. But questions and messages may also be posted as a reply to the Debian "Intent to Package" (ITP) bug report found at [7].

[1] http://svn.debian.org/wsvn/collab-maint/ext-maint/ojs/
[2] svn://svn.debian.org/collab-maint/ext-maint/ojs/
[3] http://wiki.debian.org/Alioth/PackagingProject
[4] https://alioth.debian.org/account/register.php
[5] https://alioth.debian.org/projects/collab-maint/
[6] svn+ssh://svn.debian.org/svn/collab-mai ... maint/ojs/
[7] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=511879

[jerico]

Hi Alec,

as we agree that we need some kind of internal 2.2.x-debian-release I propose to create a 2.2-debian branch based on the 2.2.x main branch (is this ojs2-branch-2_2_2?).

Only after changes to the debian-branch are well tested I'd merge them back to the 2.2.x main branch. I would then release the debian package directly off the 2.2.x branch. This way we keep the door open for eventual 2.2.x security releases that need to be pushed out quickly even before the debian changes have been tested. But we also make sure that the debian release will integrate all security/bug fixes that have been applied to 2.2.x in the meantime.

I am proposing the following extra folders in the 2.2.x-debian branch to accommodate the necessary unit tests and compatibility layer:
ojs2/classes/db/compat
ojs2/test

Inside the test-folder I would mirror the directory structure of the ojs2-folder on demand whenever I write a test. The test class would receive the name of the original class + Test at the end (e.g. ojs2/test/classes/db/DBConnectionTest.inc.php). This is pretty much the PHPUnit standard file layout and nomenclature.

Is there any chance that I can get direct write access to the 2.2.x-debian development branch?

Florian

[jerico]

Hi Alec,

I've now implemented the unit test infrastructure (see http://pkp.sfu.ca/bugzilla/show_bug.cgi?id=4518) and written some first tests against it (see patches in #4518 and #4357).

Florian

[jerico]

Hi Alec,

I wanted to know whether you could make a CVS branch 2.2-debian (or something similar) where I could commit my debian packaging and unit/web test changes.

The disadvantage I can see is:
- I'd need write access to CVS. I'd certainly only commit to "my" branch, of course. Yet there still is a certain risk that I do something that doesn't comply 100% with your commit rules/style.

The advantages are:
- We'd greatly reduce noise on BugZilla as I wouldn't have to upload patches all the time any more. I'd simply link my CVS changes to the bug reports concerned and report only important advanced on BugZilla.
- CVS is a better place to maintain and document changes. I currently scatter them over a lot of bug reports which not really is "best practice".
- Maintaining a set of patches is a little more cumbersome than maintaining changes in CVS although the tools I use (quilt, Mylyn) make the difference rather small.
- We avoid directly applying complex changes to 2.2.3. This keeps the door open for a 2.2.4 security patch release if such a thing becomes necessary. Once the 2.2-debian branch is well tested it can be merged back into the main 2.2 branch as it doesn't introduce any API-breaking changes.

Florian

[asmecher]

Hi Florian,

Yes, I think we can arrange CVS commit access -- one possible complication, though: we are occasionally making commits to the current "stable" (ojs2-branch-2_2_2) branch; if we created another branch from that for your Debian support work, we would need to ensure that those two branches were synced.

Regards,
Alec Smecher
Public Knowledge Project Team

[jerico]

one possible complication, though: we are occasionally making commits to the current "stable" (ojs2-branch-2_2_2) branch; if we created another branch from that for your Debian support work, we would need to ensure that those two branches were synced.

No need for you to do this. We don't need these changes in the Debian-branch as it only exists to isolate the Debian-changes from the stable branch as long as they are not fully tested. The Debian-branch can simply be merged back to stable as soon as we are confident (based on thorough testing) that it won't break anything there.

[jerico]

Just to document the current state of affairs: We now have a new branch "ojs2-branch-2_2_3+-debian", the branch's root is tagged as "ojs2-base-2_2_3+-debian".

All changes that have been done so far as patches have now been committed to ojs2-branch-2_2_3+-debian.