Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#8963 closed defect (fixed)

Restore compatibility with Trac 0.11

Reported by: hasienda Owned by: hasienda
Priority: high Component: AccountManagerPlugin
Severity: blocker Keywords: compatibility i18n
Cc: hju@…, rjollos 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 (18)

comment:1 Changed 3 years ago by hasienda

  • Status changed from new to assigned

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 follow-up: Changed 3 years ago by hasienda

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 3 years ago by hasienda

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 3 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 3 years ago by hasienda

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 3 years ago by rjollos

  • Cc rjollos added

comment:7 Changed 3 years ago by hasienda

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 3 years ago by hasienda

(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 3 years ago by hasienda

(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 3 years ago by hasienda

(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 3 years ago by hasienda

(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 3 years ago by hasienda

(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 3 years ago by hasienda

(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 3 years ago by hasienda

(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 3 years ago by hasienda

(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 3 years ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to closed

(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 3 years ago by hasienda

(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 3 years ago by hasienda

(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.

Add Comment

Modify Ticket

Action
as closed The owner will remain hasienda.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.