Modify

Opened 13 years ago

Closed 13 years ago

Last modified 6 years ago

#8963 closed defect (fixed)

Restore compatibility with Trac 0.11

Reported by: Steffen Hoffmann Owned by: Steffen Hoffmann
Priority: high Component: AccountManagerPlugin
Severity: blocker Keywords: compatibility i18n
Cc: hju@…, Ryan J Ollos Trac Release: 0.11

Description

It was my fault to blindly believe, that trunk has been tested by others with Trac 0.11 prior to the release.

At several occurrences I've explicitly stated that, but the true solution is still DIY. Now I've done it and I'm aware of several issue, that will immediately pop-up when acct_mgr-0.3 is applied into a 0.11 environment:

Plugin will not load due to unresolved imports, seen in Trac environment startup phase (from Trac[loader]):

  • ERROR: Skipping "acct_mgr.admin = acct_mgr.admin": (can't import "cannot import name to_utimestamp")
  • Skipping "acct_mgr.db = acct_mgr.db": (can't import "cannot import name ChoiceOption")

Plugin depends on more unavailable methods and attributes:

  • AttributeError: 'LoginModule' object has no attribute '_referer'
  • UndefinedError: "dgettext" not defined

The second list is most probably not final, since I'm still in the process of resolving the issue listed as the last one above. If you're aware of one, please add it below, preferably with a short description where to see and how to reproduce the error.

I'm sorry, and many thanks in advance for your patience. I'll try to resolve this ASAP.

Attachments (0)

Change History (19)

comment:1 Changed 13 years ago by Steffen Hoffmann

Status: newassigned

Just some preliminary comments to the issues mentioned above:

import of u_timestamp - fixed

This is an issue with the most recent addition - AccountGuard - only. It's already resolved by a conditional import with fallback to the older method to_timestamp.

import of ChoiceOption - fixed

This is resolved even more easily as the import is totally unnecessary, and was introduced with changeset [9274] for no better reason than dumb copy-n-paste by myself. I'll just remove it again.

no attribute '_referer' in trac.LoginModule - fixed

This attribute was create during development cycle for Trac 0.12, so I'll use the previous way of extracting it from req.get_header('Referer'), if required.

Still working to bring in missing dgettext.

comment:2 Changed 13 years ago by Steffen Hoffmann

UndefinedError: "dgettext" not defined - fixed

This method is simply not present before some revision of Trac 0.12dev, so I'll include code to mimic it's behavior, if required. Shameless copy-n-adapt from Trac sources here.

TypeError: __call__() got an unexpected keyword argument 'user' - fixed

Another case of missed instance while doing a rename user -> username.

AttributeError: 'LoginModule' object has no attribute 'auth_cookie_path' - fixed

This option is explicitely marked since 0.12 in the source, no wonder... Removing choices for setting the value works fine.

TypeError: gettext_noop() takes exactly 1 non-keyword argument (3 given) - fixed

Singular/plural translations do havoc here. Turned out, that the fallback assignment ngettext = _ is not sufficient, if _ alias gettext is replaced by gettext_noop, so another fallback for ngettext is requrired.

ProgrammingError: Cannot operate on a closed cursor. - fixed

Actually this points at returning a cursor object, that should be avoided in general, since the cursor get's too easy out of scope when being passed over to another instance. Wonder why later Trac releases handled this more gracefully, but let's correct this anyway: Immediately reading out the cursor and and returning the list object constructed that way is enough.

Oh, and #3323 surfaces again. No wonder, since it relies on fixed Trac 0.12 redirection logic. Now it's getting worse.

comment:3 in reply to:  2 Changed 13 years ago by Steffen Hoffmann

Replying to hasienda:

TypeError: __call__() got an unexpected keyword argument 'user' - fixed

Another case of missed instance while doing a rename user -> username.

Wrong. It turned out to be an issue with tag_ being not capable of doing a string replacement for some reason, so has to be replaced with tag(_(.

Oh, and #3323 surfaces again. No wonder, since it relies on fixed Trac 0.12 redirection logic. Now it's getting worse.

Confusing indeed. Still figuring out how to apply fixes included in recent Trac revisions into AcctMgr for working around known redirection issues of older Trac releases without destroying the current sane state for Trac >= 0.12 again.

comment:4 Changed 13 years ago by hju@…

Cc: hju@… added; anonymous removed

thx for your quick reaction.

I just tried to install 0.3 (the Stable Version) under trac 0.11 - with the detected problems.

Perhaps it would be a good idea to disable/remove the 0.11 version from the download area until the problem are solved?

Hopefully this will not take to long...

comment:5 Changed 13 years ago by Steffen Hoffmann

I'm testing redirect fixes here. Anything else is ready and will be published within the next 48 hours. So I'm reluctant to work on extra commits right now.

I'm looking forward to receiving your feedback for acct_mgr-0.3.1 then.

comment:6 Changed 13 years ago by Ryan J Ollos

Cc: Ryan J Ollos added

comment:7 Changed 13 years ago by Steffen Hoffmann

AttributeError: 'Environment' object has no attribute 'secure_cookies' - fixed

You'll be doomed, as soon as you check the "Remember me" box at AcctMgr's login page. As t:#8684 states, "AccountManagerPlugin expects Trac 0.11.2 or higher..." for a long time now. But I've not been aware of this restriction, so literally stumbled upon it while testing with 0.11, and I see, this can be fixed to work more graceful with that older Trac releases.

The nasty redirect issue is fixed too, even if I'm sure that it could be done in a cleaner fashion. Now I prepare for the real thing: release of changes to trunk and release of patched 0.3 as acct_mgr-0.3.1.

comment:8 Changed 13 years ago by Steffen Hoffmann

(In [10415]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8963.

Use to_timestamp for older Trac versions without the mircosecond POSIX time stamps support (available since Trac 0.12).

comment:9 Changed 13 years ago by Steffen Hoffmann

(In [10416]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8963.

From pwhash.py remove import of ChoiceOption introduced in changeset [9274] without real demand.

comment:10 Changed 13 years ago by Steffen Hoffmann

(In [10419]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8963.

Getting redirection right with Trac 0.11 is quite tricky. First: extract referer from req.get_header('Referer'), if (the inherited) trac.LoginModule has no attribute '_referer'.

comment:11 Changed 13 years ago by Steffen Hoffmann

(In [10421]) AccountManagerPlugin: Second: Improve redirect loop protection - again, refs #3233 and #8963.

We already knew a lot about infinite loops starting from '/login' (see #3233), but for Trac 0.11 obviously this has been not enough. Still I wonder, why no one complained about it recently.

Hint: Trac 0.11 doesn't preserve the referer very well, but I'm reluctant to fix this in AccountManager. Better use a current Trac, if you dislike it.

comment:12 Changed 13 years ago by Steffen Hoffmann

(In [10422]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8963.

Recent templates use dgettext to reliably extract and translate special content like button labels. Because for the 0.11 Trac releases this isn't included into default content of the data object passed to Genshi alongside with the template, this has to be added explicitely to each and every data object created with this plugin.

More importantly, there is some code missing to degrade more complex i18n calls like dgettext and ngettext gracefully for Trac 0.11, so more code has been taken and adapted from current Trac trunk to fill in the gap here.

comment:13 Changed 13 years ago by Steffen Hoffmann

(In [10423]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8684 and #8963.

Replace tag_ with more capable tag(_( call to prevent TypeError in Genshi used by any Trac 0.11 release. But this is generally a good idea, i.e. if looking at #8684.

comment:14 Changed 13 years ago by Steffen Hoffmann

(In [10426]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8963.

auth_cookie_path option is explicitely marked since 0.12 in the source, my own fault to access it unconditionally, sorry.

comment:15 Changed 13 years ago by Steffen Hoffmann

(In [10428]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8963.

Lift unnecessary restriction regarding the secure_cookies option, a feature of Trac 0.11.2 and later, to restrict cookies to HTTPS connections only. And again I refrain from the tempting idea to fix it in AcctMgr itself. The cookie creation isn't totally independed of Trac, and I'm not an expert to spot possible side-effects of such a move.

comment:16 Changed 13 years ago by Steffen Hoffmann

Resolution: fixed
Status: assignedclosed

(In [10429]) AccountManagerPlugin: Merge relevant changes between [10415] and [10428], closes #8963.

Push recent fixes and other changes from trunk to 0.11 branch for a big gain in backwards-compatibility with Trac 0.11 and later (before Trac 0.12). Still there are some bugs that I'm aware of, but these are not the major show-stopper, that had to be taken down immediately. Thanks for your patience.

comment:17 Changed 13 years ago by Steffen Hoffmann

(In [10467]) AccountManagerPlugin: Restore 0.11 compatibility, refs #8963.

Trac 0.11 produces duplicated HTTP request arguments when sending POST request of account details admin page received with GET arguments before. Now explicitly setting an action URL for POST results in consistent behavior.

And another occurance of tag_ is replaced here, to get rid of TypeError (unexpected keyword argument 'user') again.

comment:18 Changed 13 years ago by Steffen Hoffmann

(In [10470]) AccountManagerPlugin: Publish maintenance release 0.3.1, refs #8963.

Shortly after starting my own Trac 0.11 compatibility testing the issues, that popped up immediately, are completely resolved. Given low feedback so far by tickets and ticket comments, it seems like a comprehensive fixture. Enjoy.

comment:19 Changed 6 years ago by Ryan J Ollos

In 17228:

TracAccountManager 0.5.1dev: Remove compatibility code

Trac < 1.0 is no longer supported. This reverts r10421.

Refs #3233, #8963.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.