Release Trac 1.0

[Remy Blank]

No open tickets on 1.0 anymore, so it's time to think about releasing! We still have a few things to tune and improve, but we're in a good state to release a beta1 Real Soon Now^TM.

This ticket will be used to coordinate the finalization, testing and releasing of Trac 1.0.

Testing

People having volunteered to contribute to release testing on TracDev/ReleaseTesting were added to the CC: list.

If you'd like to contribute, please fill an entry in the table you'll find there, detailing the configuration(s) you will test, and report the status of your testing in this ticket.

Status

1.0beta1 → r11191
1.0rc1 → r11242
1.0 → September 7, 2012 r11307

[Christian Boos]

Early feedback did show two rough edges in 1.0dev when upgrading from a Trac 0.12.x environment:

1. the move of the svn related components to tracopt. imply that they have to be explicitly enabled now if they're needed
2. the migration of attachments can fail in its last step, if they are leftover files in the old attachments folder

Dealing with 1. is probably a matter of writing an environment upgrader component which:

1. looks if svn is explicitly enabled or disabled (hm, hope we can determine this from the config)
2. if not set either way, look if there's if there's one repository with the legacy svn repository type (svn, svnfs, direct-svnfs) and if yes, enable the components

DONE r11185

For 2., it might be as simple as catching the error on attachments dir removal and displaying a clear error message (e.g. "The '%(tracenv)s/attachments/' directory couldn't be removed. This can happen if it contains files that are no longer referenced as attachments. Please move these files away and upgrade again.")

DONE r11162

[Remy Blank]

Yes, these were the two things I had in mind, too. For 2., I would go one step further and rename the attachments directory to stale_attachments or old_attachments (or a variant containing the current date), and mention the new name in the post-upgrade message. This makes the upgrade succeed, there's no need to re-run trac-admin $ENV upgrade, and the installation should be functional. And even if the admin doesn't read the message, the stale files are still there and safe.

There's one more cosmetic change that I would like to suggest: can I make the font of ticket subjects slightly smaller, and add a clear: right style to the heading? The current font makes many subjects wrap, and I find it difficult to read. And with the current layout, it wraps even earlier due to the "Opened" and "Last modified" lines (this is due to my font settings, but I believe I can fix it without making it worse for everyone else). Suggested patch coming shortly.

[Remy Blank]

Oh, wait, the larger font is a customization here on t.e.o, the standard font is fine. The clear: right would still help, though.

[Remy Blank]

Always place the ticket subject below the "Opened" and "Last modified" times.

[Remy Blank]

10779-ticket-title-r11150.patch​ looks fine on all browsers with and without setting a minimum font size. Thoughts?

[Christian Boos]

Yep, when I played with clear:right after your suggestion, I also removed the negative bottom margin, so fine for me with this change!

[Remy Blank]

Applied in [11151].

(Also, since we're still tuning, and don't actually have a package to test yet, I have temporarily removed the testers from CC so that they don't get spammed. I'll re-add them when beta1 is ready.)

[Jun Omae]

Double scrollbars are present on IE8 compatibility mode

[Jun Omae]

Patch for double scrollbars are present on IE8 compatibility mode

[Jun Omae]

Double scrollbars are present on IE8 compatibility mode, double-scrollbars-ie8-compat-mode.png​. It works well on IE8 standard mode. I think we should move overflow-y: scroll to html from body to solve the issue and use X-UA-Compatible: IE=edge because Trac 1.0 doesn't support IE7 (CSS engine on IE7 is buggy!).

double-scrollbars-ie8-compat-mode-r11154.diff​ works fine on IE7-8, Firefox 14, Chrome 20, Safari 5 and Opera 12.

[Christian Boos]

Replying to jomae:

• move overflow-y: scroll to html from body to solve the issue

Sure, I saw recipes using html, but as in my testing it worked well with body, I thought that would be a better semantic fit. Forgot to test with IE7 obviously ;-)

use X-UA-Compatible: IE=edge because Trac 1.0 doesn't support IE7 (CSS engine on IE7 is buggy!).

Are there other problems with IE7 besides the fact it doesn't look pretty?

double-scrollbars-ie8-compat-mode-r11154.diff​

Please apply!

[Jun Omae]

Problems in wiki page and fat buttons on IE7

[Jun Omae]

Replying to cboos:

Are there other problems with IE7 besides the fact it doesn't look pretty?

Two problems. wiki-horizontal-scrollbar-fat-buttons.png​.

1. Wiki page always has horizontal scroll bar and no contents on the scrolled area. I have no idea to solve.
2. Too much margin on button. The inline buttons (e.g. "↳ Reply") have the same issue.
   It would be nice to add the following to trac.css to solve. See ​http://stackoverflow.com/a/7718010/709074.

   trac/htdocs/css/trac.css

   @@ -83 +83 @@
    /* Forms */
    input, textarea, select { margin: 2px }
    input, select { vertical-align: middle }
   +input[type=button], input[type=submit], input[type=reset], button {
   + *overflow: visible; /* Workaround too much margin on button in IE7 */
   +}
    input[type=button], input[type=submit], input[type=reset] {
     background: #eee;
     color: #222;

double-scrollbars-ie8-compat-mode-r11154.diff​

Please apply!

Applied in [11155].

[Christian Boos]

Replying to jomae:

1. Wiki page always has horizontal scroll bar and no contents on the scrolled area. I have no idea to solve.

Try: html { overflow-x: hidden } (just for IE7, so probably have * in front, IIRC)

2. Too much margin on button. The inline buttons (e.g. "↳ Reply") have the same issue.
   It would be nice to add the following to trac.css to solve…

Proposed fix seems to work fine!

[Jun Omae]

Replying to cboos:

Replying to jomae:

1. Wiki page always has horizontal scroll bar and no contents on the scrolled area. I have no idea to solve.

Try: html { overflow-x: hidden } (just for IE7, so probably have * in front, IIRC)

It will get a problem if the wiki page really needs horizontal scrollbar….

Then, I found the workaround. It would be nice to add *width: 100% to div.trac-modifiedby for only IE7.

2. Too much margin on button. The inline buttons (e.g. "↳ Reply") have the same issue.
   It would be nice to add the following to trac.css to solve…

Proposed fix seems to work fine!

Two workarounds have been applied in [11157].

[Christian Boos]

Replying to cboos:

1. the move of the svn related components to tracopt. imply that they have to be explicitly enabled now if they're needed (…)

Dealing with 1. is probably a matter of writing an environment upgrader component which:

1. looks if svn is explicitly enabled or disabled (hm, hope we can determine this from the config)
2. if not set either way, look if there's if there's one repository with the legacy svn repository type (svn, svnfs, direct-svnfs) and if yes, enable the components

See repos:cboos.git:ticket10779-tracopt-svn-auto-enabler

(mh, scratch the required = true line)

[Remy Blank]

I don't think I'd want that. Sure, environment_needs_upgrade() is only called when creating an environment, so the performance impact is probably negligible. But this component will remain active forever (unless explicitly disabled, which nobody will do), so when at some point you want to disable SVN e.g. for debugging purposes, it will re-add it automatically.

It's also missing a number of variants in the way SVN can be enabled in tracopt.

Then, updating trac.ini even when the backup copy failed feels wrong (why make a copy in the first place, then?). Not to mention that it will remove the comments that I keep in trac.ini (no, I never edit settings in the admin panel).

What's wrong with a big fat "If you use Subversion, add this line to the [components] section of your trac.ini: tracopt.versioncontrol.svn.* = enabled" in the release notes?

[Christian Boos]

Replying to rblank:

I don't think I'd want that. Sure, environment_needs_upgrade() is only called when creating an environment, so the performance impact is probably negligible. But this component will remain active forever (unless explicitly disabled, which nobody will do),

so when at some point you want to disable SVN e.g. for debugging purposes, it will re-add it automatically.

No, if you set it to disabled, it will stay disabled. It's only enabled automatically when there's nothing set in [components] and when there's a repository of the svn type, so something that can be useful anytime, not just during the 0.12 → 1.0 upgrade.

It's also missing a number of variants in the way SVN can be enabled in tracopt.

Like?

Then, updating trac.ini even when the backup copy failed feels wrong (why make a copy in the first place, then?). Not to mention that it will remove the comments that I keep in trac.ini (no, I never edit settings in the admin panel).

The copy is made to save the comments, until we have #7378. Admittedly, we don't do that when saving the config after web admin changes, but as you said, seasoned Trac users know that they have to avoid doing this if they want to keep their comments, while for an automatic upgrade we never wrote to the config so far, so people might be caught unaware. Better be on the safe side and we could even stop the upgrade in case of failure, a bit like what we did for the attachment upgrade, yes.

What's wrong with a big fat "If you use Subversion, add this line to the [components] section of your trac.ini: tracopt.versioncontrol.svn.* = enabled" in the release notes?

Nobody reads the TracUpgrade ;-) Ok, not nobody, but when you have at least two people from the TracTeam getting caught by this thing when they should have known better (eblot, and yes, even me ;-) ), it's a good indication that you should assist the process with some automation instead of relying only on the documentation.

[Remy Blank]

Replying to cboos:

No, if you set it to disabled, it will stay disabled.

Removing or commenting out the line tracopt.versioncontrol.svn_fs.* = enabled will disable it, and it will be re-added automatically.

Like?

tracopt.versioncontrol.svn.svn_fs.SubversionConnector = enabled
tracopt.versioncontrol.* = enabled
tracopt.versioncontrol = enabled
tracopt.* = enabled
tracopt = enabled

Admittedly, we don't do that when saving the config after web admin changes, but as you said, seasoned Trac users know that they have to avoid doing this if they want to keep their comments,

Yes, while they don't suspect anything like that on upgrade.

it's a good indication that you should assist the process with some automation instead of relying only on the documentation.

It's a good indication that people should handle their upgrades more seriously, or maybe that we shouldn't have moved the modules to tracopt. It's hardly a good justification for adding more magic.

[Remy Blank]

There's actually a subtle technical issue with this upgrade (and now that I think of it, the attachment upgrade is affected as well).

We don't order calls to IEnvironmentSetupParticipant instances. Imagine that in the future, we change the schema for the repository table. Then TracOptSubversionAutoEnabler.environment_needs_upgrade() gets called before the DB upgrade. It will instantiate the new RepositoryManager, which expects the repository table to have the new schema, and it will probably crash. We had this issue in the upgrader for multirepos (#9252), which led in [9517] to use SQL only.

This leads to at least two conclusions:

• We should always have a strict ordering of upgrades. The simple way to do this is to have all our upgrades in trac.upgrades, even non-DB ones, and never have a separate IEnvironmentSetupParticipant in core.
• We can rely on very few things in upgrades. In particular, components are at the latest version during an upgrade, so upgrades relying on components can easily break when the components change. I would suggest that we should only ever rely on the environment object, the configuration, the database connection, and utility functions explicitly marked as being used for upgrades (indicating that any changes to these functions should be checked against upgrades).

The attachment upgrade has the same issue: it will probably break when we change the attachment table, because it uses the Attachment model class, which will assume the new schema is already in place. I would suggest rewriting the attachment upgrade as a DB upgrade, which will also avoid having to move the old attachments directory (and possibly failing).

For the auto-enabling of tracopt.versioncontrol.svn, I would suggest keeping it simple. How about adding a DB upgrade step that checks if trac.versioncontrol.svn_fs (and a few limited variants) is explicitly disabled in trac.ini, and if it isn't, to auto-enable tracopt.versioncontrol.svn? This will ensure that new installs don't auto-enable, and upgrades auto-enable once, unless explicitly disabled. The backup copy is nice, but I noticed that we already had upgrades that re-wrote trac.ini without a backup in the past, so it's not even mandatory.

Thoughts?

[Christian Boos]

Replying to rblank:

(…) I would suggest rewriting the attachment upgrade as a DB upgrade, which will also avoid having to move the old attachments directory (and possibly failing).

But leaving the old attachment folder around would be confusing. I think we should try to move it anyway.

For the auto-enabling of tracopt.versioncontrol.svn, I would suggest keeping it simple. How about adding a DB upgrade step that checks if trac.versioncontrol.svn_fs (and a few limited variants) is explicitly disabled in trac.ini, and if it isn't, to auto-enable tracopt.versioncontrol.svn? This will ensure that new installs don't auto-enable, and upgrades auto-enable once, unless explicitly disabled. The backup copy is nice, but I noticed that we already had upgrades that re-wrote trac.ini without a backup in the past, so it's not even mandatory.

I was about to suggest that the TracOptSubversionAutoEnabler could disable itself, so that it couldn't enable tracopt.versioncontrol.svn more than once, but the solution you propose is also fine, it seems. The problem with the RepositoryManager was well spotted!

[Remy Blank]

Replying to cboos:

Replying to rblank:

(…) I would suggest rewriting the attachment upgrade as a DB upgrade, which will also avoid having to move the old attachments directory (and possibly failing).

But leaving the old attachment folder around would be confusing. I think we should try to move it anyway.

I am currently working on fixing the attachment upgrade (in db28.py, so please choose db29.py when you continue on this ;)

What I planned to do was to try to delete the old "attachments" directory, and if it fails, to print a message and leave it there. Patch coming shortly.

[Christian Boos]

Replying to rblank:

This leads to at least two conclusions:

• We should always have a strict ordering of upgrades. The simple way to do this is to have all our upgrades in trac.upgrades, even non-DB ones, and never have a separate IEnvironmentSetupParticipant in core.

That would probably deserve a comment next to IEnvironmentSetupParticipant.

• We can rely on very few things in upgrades. In particular, components are at the latest version during an upgrade, so upgrades relying on components can easily break when the components change. I would suggest that we should only ever rely on the environment object, the configuration, the database connection, and utility functions explicitly marked as being used for upgrades (indicating that any changes to these functions should be checked against upgrades).

I wonder to which extent those caveats also apply to the implementations of IEnvironmentSetupParticipant by plugins. If they use at some point any Trac component, they must be sure that Trac itself has completed its upgrade. This suggests that we should order the IEnvironmentSetupParticipant according to the package dependencies.

[Remy Blank]

Replying to cboos:

That would probably deserve a comment next to IEnvironmentSetupParticipant.

Good idea, I'll add that.

I wonder to which extent those caveats also apply to the implementations of IEnvironmentSetupParticipant by plugins. If they use at some point any Trac component, they must be sure that Trac itself has completed its upgrade. This suggests that we should order the IEnvironmentSetupParticipant according to the package dependencies.

Yes, that's probably true. I'm not sure how easy it would be to order by dependencies (and what about circular dependencies?), but we could at least upgrade Trac core first, before upgrading plugins. This is probably the case already, as the IEnvironmentSetupParticipant in env.py is likely the first one to be registered, and we keep them in a list.

[Remy Blank]

Move the attachment upgrade into the DB upgrade infrastructure.

[Remy Blank]

10779-attachment-upgrade-r11167.patch​ moves the attachment upgrade into the DB upgrade infrastructure. I have tested various edge cases and it seems to be working fine, but more testing is always welcome.

[osimons]

Replying to rblank:

This leads to at least two conclusions:

• We should always have a strict ordering of upgrades. The simple way to do this is to have all our upgrades in trac.upgrades, even non-DB ones, and never have a separate IEnvironmentSetupParticipant in core.
• We can rely on very few things in upgrades. In particular, components are at the latest version during an upgrade, so upgrades relying on components can easily break when the components change. I would suggest that we should only ever rely on the environment object, the configuration, the database connection, and utility functions explicitly marked as being used for upgrades (indicating that any changes to these functions should be checked against upgrades).

Huh, this all sounds somewhat familiar… Ah, yes, it is the comment:35:ticket:10313 discussion. Not surprisingly I'm in favour of a much more predictable upgrade handling.

In the Bitten project we also had similar issues with changes and assumptions that over time proved not to hold true for some upgrades / sequences of upgrades. In the end, Hodgestar included upgrades in our tests too, so that we actually create a version 1 of the Bitten schema and call each upgrade in succession to (hopefully) arrive at the current schema / config / file / sequence structure for the Trac+Bitten combination being tested. Very useful, see ​bitten:source:/trunk/bitten/tests/upgrades.py

[Remy Blank]

Handle case where attachments directory doesn't exist.

[Remy Blank]

10779-attachment-upgrade-2-r11168.patch​ is a slightly improved version that correctly handles the case where the attachments directory doesn't exist (e.g. dev installs that were already upgraded by the previous upgrader).

[Remy Blank]

Replying to osimons:

Huh, this all sounds somewhat familiar… Ah, yes, it is the comment:35:ticket:10313 discussion. Not surprisingly I'm in favour of a much more predictable upgrade handling.

Yes, indeed. It would have helped at the time if you had had a slightly more technical argumentation in favor of the DB upgrade scheme than "Is it just me being dumb when I don't quite understand why this is the best way?", but yes, you were right ;)

[Christian Boos]

Replying to rblank:

10779-attachment-upgrade-2-r11168.patch​ is a slightly improved version that correctly handles the case where the attachments directory doesn't exist (e.g. dev installs that were already upgraded by the previous upgrader).

Thanks, good timing, as I was just about to test that on top of previously migrated test environments ;-)

Works well, one minor nit with the following output:

The upgrade of attachments was successful, but the old attachments directory:

  C:\Trac\envs\testUpgr5\attachments

couldn't be removed, possibly due to the presence of files that weren't
referenced in the database. The error was:

  WindowsError: [Error 145] The directory is not empty: 'C:\\Trac\\envs\\testUpgr5\\attachments\\wiki\\WikiStart'

Upgrade done.

You may want to upgrade the Trac documentation now by running:

  trac-admin C:\Trac\envs\testUpgr5 wiki upgrade

I think that after seeing this, the user will wonder what to make with this error, if it's a real problem or something that can be ignored or…

So I think an additional line of guidance would be welcome:

... The error was:

  WindowsError: [Error 145] The directory is not empty: 'C:\\Trac\\envs\\testUpgr5\\attachments\\wiki\\WikiStart'

This error can be ignored, but for keeping your environment clean
you should remove the old attachments folder manually.

Upgrade done.

Or something like that.

[Remy Blank]

Replying to cboos:

So I think an additional line of guidance would be welcome:

Sounds good, I'll integrate that sentence. Other than that, OK to commit?

[Christian Boos]

Replying to rblank:

Replying to cboos:

So I think an additional line of guidance would be welcome:

Sounds good, I'll integrate that sentence. Other than that, OK to commit?

Sure! I'll find some time today to do db29.py and then we're ready for the beta this evening.

[Remy Blank]

Applied in [11169] and [11170].

[Christian Boos]

See repos:cboos.git:ticket10779-tracopt-svn-db29 (enable tracopt.versioncontrol.svn.* with a new db upgrade).

[Christian Boos]

(all other release coordination tickets are set to task)

[osimons]

Replying to cboos:

See repos:cboos.git:ticket10779-tracopt-svn-db29 (enable tracopt.versioncontrol.svn.* with a new db upgrade).

Almost, but I think you should also do a check to see if tracopt.versioncontrol.svn. SubversionConnector component is already enabled, and not make the change if it is. Being aware of the change I would naturally implement the required settings in a shared config before upgrading, and not risk all projects getting local modifications through the upgrade code.

[Remy Blank]

Replying to osimons:

Almost, but I think you should also do a check to see if tracopt.versioncontrol.svn. SubversionConnector component is already enabled, and not make the change if it is.

+1

[Christian Boos]

Replying to rblank:

Replying to osimons:

Almost, but I think you should also do a check to see if tracopt.versioncontrol.svn. SubversionConnector component is already enabled, and not make the change if it is.

+1

Sure, here it is: [d35c3c4a/cboos.git] Do nothing if the new components are already enabled.

[Christian Boos]

One more:

• [920dca37/cboos.git] Consider every svn component, not just .svn_fs.SubversionConnector

If someone could test and confirm this works well enough, I'll be able to commit and begin packaging the beta ;-)

[Jun Omae]

Replying to cboos:

If someone could test and confirm this works well enough, I'll be able to commit and begin packaging the beta ;-)

It works fine!

BTW, trac-admin command doesn't check the needs for upgrades. I think it should check it. Otherwise, the sub commands of trac-admin can be executed on the environment without the upgrades.

[Jun Omae]

BTW, trac-admin command doesn't check the needs for upgrades. I think it should check it. Otherwise, the sub commands of trac-admin can be executed on the environment without the upgrades.

[7be7fa25/jomae.git] checks the needs to be upgraded and shows the upgrade message before executing commands except initenv and help. Thoughts?

[Christian Boos]

Replying to jomae:

[7be7fa25/jomae.git] checks the needs to be upgraded and shows the upgrade message before executing commands except initenv and help. Thoughts?

$ PYTHONPATH=. python trac/admin/console.py ... upgrade
Error: The Trac Environment needs to be upgraded.

Run "trac-admin ... upgrade"

:-)

[Jun Omae]

Ouch! That's so bad… Pushed again. [fbad1b19/jomae.git] and [34b867869/jomae.git].

[Christian Boos]

Replying to jomae:

Ouch! That's so bad… Pushed again. [fbad1b19/jomae.git].

Yes, I came with nearly the identical fix, except for testing for not in ['help', 'upgrade'] and using AdminCommandError. But as help upgrade doesn't give much information, it's not that important and your approach with [34b867869/jomae.git] will also do.

[Jun Omae]

Replying to cboos:

Yes, I came with nearly the identical fix, except for testing for not in ['help', 'upgrade'] and using AdminCommandError. But as help upgrade doesn't give much information, it's not that important and your approach with [34b867869/jomae.git] will also do.

Applied in [11184].

[Christian Boos]

Replying to jomae:

Replying to cboos:

If someone could test and confirm this works well enough, I'll be able to commit and begin packaging the beta ;-)

It works fine!

Thanks for the feedback, applied in r11185.

Now I'll wait until this evening for the green light of Remy before tagging.

Still a few fixes and an extraction to make in the meantime. Would it be worth to make update as well?

[Remy Blank]

Works fine here. Ready to go for testing!

[Christian Boos]

Replying to rblank:

Works fine here. Ready to go for testing!

Ok, thanks for the feedback!

[Christian Boos]

Replying to self:

Still a few fixes and an extraction to make in the meantime. Would it be worth to make update as well?

For the language update, I think it's better to pass and let each translator do it: we currently have no errors when doing make compile, but we would have a bunch after make update. Example for make update-fr compile-fr:

running compile_catalog
error: trac/locale\fr\LC_MESSAGES\messages.po:3079: unknown named placeholder u'change'
error: trac/locale\fr\LC_MESSAGES\messages.po:3640: unknown named placeholder u'verb'
error: trac/locale\fr\LC_MESSAGES\messages.po:3645: unknown named placeholder u'verb'
error: trac/locale\fr\LC_MESSAGES\messages.po:3650: unknown named placeholder u'verb'
error: trac/locale\fr\LC_MESSAGES\messages.po:3655: unknown named placeholder u'verb'
error: trac/locale\fr\LC_MESSAGES\messages.po:3660: unknown named placeholder u'id'
error: trac/locale\fr\LC_MESSAGES\messages.po:3999: unknown named placeholder u'parent'
error: trac/locale\fr\LC_MESSAGES\messages.po:5554: unknown named placeholder u'page'
catalog 'trac/locale\\fr\\LC_MESSAGES\\messages.po' contains errors, skipping

Fixing all of these would be tedious and error prone, so better have missing messages.

Besides, I even tried make check from #10569 with the current trunk and it seems OK (we should really have integrated it!).

[Christian Boos]

Ok, the beta1 packages are now available in TracDownload#LatestBeta. I did some testing of the Windows installer, no surprise there, everything seems to work fine. The present Trac instance is also running the beta1 now.

Adding back the list of testers (if some of you are not interested in testing this release, please remove your entry TracDev/ReleaseTesting, thanks!).

I'll write a mail to Trac-Dev tomorrow, if nobody beats me at it ;-) Should we also send one to Trac-users?

[Remy Blank]

As one (small) data point, I upgraded my personal Trac install, and everything works great. Enabling SVN in tracopt correctly prevented my trac.ini from being messed up, and attachments were moved correctly. Re-styling the few parts that I don't like in the new UI style was easy, too.

Oh, and I sent a short email to both trac-dev and trac-users. No point in letting our users wait :) Thanks for packaging the release, Christian.

[Thijs Triemstra]

Can we start updating the translations for this release (and will they still be able to go in 1.0)?

[Christian Boos]

Replying to thijstriemstra:

Can we start updating the translations for this release (and will they still be able to go in 1.0)?

Sure! We'll wait a reasonable amount of time for the translations to come in, but the sooner the better, of course. As mentioned on Trac-dev, we're in string freeze now, so no changes on the text source should happen.

[Samuel.Degrande@…]

( I don't really know where to submit this patch, but I saw another patch for a css issue on this page, so I permit myself to submit it here. Just flame me if I was not granted to do so… )

When you click on a 'Add' button in a form.addnew, the button is translated much too far away. Here is a proposed patch (for r11195):

admin.css

@@ -60 +60 @@
  padding: 0.2em 0.5em 0.2em 0;
  white-space: nowrap;
 }
-form.addnew div.buttons input { margin: 0 0.5em 0 0; }
+form.addnew div.buttons input { margin: 0 0.5em .1em 0; }
+form.addnew div.buttons input[type=button]:active,
+form.addnew div.buttons input[type=submit]:active,
+form.addnew div.buttons input[type=reset]:active {
+ margin: .1em .4em 0 0.1em;
+}
+
 form.addnew p.hint,
 form.addnew span.hint {
  padding-left: 0.5em;

[Samuel.Degrande@…]

Replying to Samuel.Degrande@…:

( I don't really know where to submit this patch, but I saw another patch for a css issue on this page, so I permit myself to submit it here. Just flame me if I was not granted to do so… )

Oops. I'm really sorry. I did not see that it was fixed in r11195…

Can someone just remove my previous comment.

Sorry again…

[Christian Boos]

Replying to Samuel.Degrande@…:

Oops. I'm really sorry. I did not see that it was fixed in r11195…

No worries, and thanks for the patch anyway! Next time ;-)

[Itamar Oren]

I tried a x64 installation (Win7-x64, Python-2.7.3-x64), and ran tests on the beta from inside virtualenv. Got 2 F's on functional tests:

(vpy27-trac10-beta) C:\Users\Itamar\Workspaces\TracSpace\vpy27-trac10-beta\Trac-1.0beta1>python -m trac.test
SKIP: validation of XHTML output in functional tests (no lxml installed)
SKIP: versioncontrol/tests/functional.py (no svn bindings)
SKIP: tracopt/mimeview/tests/php.py (php cli binary, 'php', not found)
SKIP: tracopt/versioncontrol/git/tests/PyGIT.py (git cli binary, 'git', not found)
SKIP: tracopt/versioncontrol/svn/tests/svn_fs.py (no svn bindings)
.....................................................................................................................F................................
................................................F.....................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
.......................................
======================================================================
FAIL: runTest (trac.ticket.tests.functional.TestTicketQueryLinks)
Test ticket query links
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac\ticket\tests\functional.py", line 202, in runTest
    tc.find('class="missing">← Previous Ticket')
  File "trac\tests\functional\better_twill.py", line 204, in better_find
    raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ('no match to \'class="missing">← Previous Ticket\'', 'C:\\Users\\Itamar\\Workspaces\\TracSpace\\vpy27-trac10-beta\\Trac-1.0
beta1\\testenv\\trac\\log\\TestTicketQueryLinks.html')

======================================================================
FAIL: runTest (trac.prefs.tests.functional.TestPreferences)
Set preferences for admin user
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac\prefs\tests\functional.py", line 24, in runTest
    tc.notfind('Your preferences have been saved.')
  File "trac\tests\functional\better_twill.py", line 212, in better_notfind
    raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ("match to 'Your preferences have been saved.'", 'C:\\Users\\Itamar\\Workspaces\\TracSpace\\vpy27-trac10-beta\\Trac-1.0beta1\\tes
tenv\\trac\\log\\TestPreferences.html')

----------------------------------------------------------------------
Ran 1239 tests in 211.973s

FAILED (failures=2)

[Remy Blank]

This seems to be Windows-specific. All tests pass here on Linux but I get even more failures (and even one error) on Windows 7 32-bit with Python 2.6.4.

======================================================================
ERROR: Test for regression of http://trac.edgewall.org/ticket/6048
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\users\joe\src\trac\trunk\trac\ticket\tests\functional.py", line 1369,
 in runTest
    summary='RegressionTestTicket6048')
  File "c:\users\joe\src\trac\trunk\trac\tests\functional\tester.py", line 66, i
n create_ticket
    tc.follow('New Ticket')
  File "C:\Python26\lib\site-packages\twill-0.9-py2.6.egg\twill\commands.py", li
ne 196, in follow
    link = browser.find_link(regexp)
  File "C:\Python26\lib\site-packages\twill-0.9-py2.6.egg\twill\browser.py", lin
e 182, in find_link
    l = self._browser.find_link(url_regex=pattern)
  File "C:\Python26\lib\site-packages\twill-0.9-py2.6.egg\twill\other_packages\_
mechanize_dist\_mechanize.py", line 626, in find_link
    return self._filter_links(self._factory.links(), **kwds).next()
  File "C:\Python26\lib\site-packages\twill-0.9-py2.6.egg\twill\other_packages\_
mechanize_dist\_mechanize.py", line 648, in _filter_links
    raise BrowserStateError("not viewing HTML")
BrowserStateError: not viewing HTML

======================================================================
FAIL: Test ticket query links
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\users\joe\src\trac\trunk\trac\ticket\tests\functional.py", line 202,
in runTest
    tc.find('class="missing">← Previous Ticket')
  File "c:\users\joe\src\trac\trunk\trac\tests\functional\better_twill.py", line
 204, in better_find
    raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ('no match to \'class="missing">← Previous Ticket\'',
'c:\\users\\joe\\src\\trac\\trunk\\testenv\\trac\\log\\TestTicketQueryLinks.html
')

======================================================================
FAIL: Set preferences for admin user
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\users\joe\src\trac\trunk\trac\prefs\tests\functional.py", line 24, in
 runTest
    tc.notfind('Your preferences have been saved.')
  File "c:\users\joe\src\trac\trunk\trac\tests\functional\better_twill.py", line
 212, in better_notfind
    raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ("match to 'Your preferences have been saved.'", 'c:\\users
\\joe\\src\\trac\\trunk\\testenv\\trac\\log\\TestPreferences.html')

======================================================================
FAIL: Test RecentChanges, group option
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\users\joe\src\trac\trunk\trac\wiki\tests\formatter.py", line 207, in
test
    % (msg, self.file, self.line, self.title, formatter.flavor))
AssertionError:
========== expected: ==========
<p>
</p><div><h3>08/18/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><h3>08/18/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li></ul></div><p>
</p>


========== actual: ==========
<p>
</p><div><h3>2012-08-18</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><h3>2012-08-18</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li></ul></div><p>
</p>


========== wiki: ==========
u'
[[RecentChanges()]]
[[RecentChanges(group=date)]]
[[RecentChanges(group=none)]]
[[RecentChanges(,2,group=none)]]
[[RecentChanges(Wiki,group=none)]]
[[RecentChanges(Wiki,1,group=none)]]
'
========== diff: ==========
--- expected
+++ actual
@@ -1,9 +1,9 @@
 <p>
-</p><div><h3>08/18/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
+</p><div><h3>2012-08-18</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
 </li><li><a href="/wiki/WikiMid">WikiMid</a>
 </li><li><a href="/wiki/WikiStart">WikiStart</a>
 </li></ul></div><p>
-</p><div><h3>08/18/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
+</p><div><h3>2012-08-18</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
 </li><li><a href="/wiki/WikiMid">WikiMid</a>
 </li><li><a href="/wiki/WikiStart">WikiStart</a>
 </li></ul></div><p>


c:\users\joe\src\trac\trunk\trac\wiki\tests\macros.py:1: "RecentChanges, group o
ption" (default flavor)

----------------------------------------------------------------------
Ran 1379 tests in 827.399s

FAILED (failures=3, errors=1)

The last failure is a l10n issue, and I thought we had a fix in place for that.

[Remy Blank]

Oh, I know where the l10n failure comes from. I have set up Windows with an en_US locale, but I have set a custom date format. So just forcing an en_US locale when running tests is not enough. Fixing this probably means mocking the date formatting code. Do we want to fix this, or is it too borderline?

[Remy Blank]

The error on the regression test for #6048 is flaky, I only get it from time to time. I blame Windows on VirtualBox.

The other two are consistent. The TestTicketQueryLinks test seems to fail because the current query isn't stored in the session. The query gives the correct number of results, but when going to one of the tickets, the query isn't "remembered". I haven't found why.

The TestPreferences also seems to point to a session storage failure, because the symptom is that the "Your preferences have been saved." message shown after saving preferences persists after going to a different pref page.

I have tried executing the same operations "by hand" in the browser, and I get the correct behavior. Weird.

[Remy Blank]

I give up for today. Both issues really smell like bad session attributes. At some point, I disabled the first failing test and the other passed! But only once, it failed again on the next run.

Any ideas?

[Christian Boos]

No errors for me on Windows 7 (not a VM) with Python 2.7.2, 2.6.5 and 2.5.4 … so no idea about what's happening so far.

[Remy Blank]

Itamar, are you running Windows in a VM (VirtualBox, by any chance)?

[Itamar Oren]

Replying to rblank:

Itamar, are you running Windows in a VM (VirtualBox, by any chance)?

No VM. Bare metal Windows 7 Ultimate x64. I think my default locale is Hebrew, which also has a different date format from en_US. Could this be the reason for failure?

[Remy Blank]

I don't see how the two failures you observed could be locale-related. But you could temporarily switch to en_US to make sure.

[Itamar Oren]

Replying to rblank:

I don't see how the two failures you observed could be locale-related. But you could temporarily switch to en_US to make sure.

I did not change the locale. Actually, I did not change anything - I just reran the tests, and..:

(vpy27-trac10-beta) C:\Users\Itamar\Workspaces\TracSpace\vpy27-trac10-beta\Trac-1.0beta1>python -m trac.test
SKIP: validation of XHTML output in functional tests (no lxml installed)
SKIP: versioncontrol/tests/functional.py (no svn bindings)
SKIP: tracopt/mimeview/tests/php.py (php cli binary, 'php', not found)
SKIP: tracopt/versioncontrol/git/tests/PyGIT.py (git cli binary, 'git', not found)
SKIP: tracopt/versioncontrol/svn/tests/svn_fs.py (no svn bindings)
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
......................................................................................................................................................
.......................................
----------------------------------------------------------------------
Ran 1239 tests in 200.544s

OK

[Remy Blank]

Oh, nice, flaky tests. They still pretty consistently fail here… except when they don't :-/

[Remy Blank]

Everything is looking pretty good so far. How about 1.0rc1 in the next few days? Thursday evening? ;)

[Christian Boos]

I really wish we could get #10569 in first and maybe then get 0.12.4rc1 first… And what about a little debug session tomorrow for these flaky tests? ;-)

[Remy Blank]

Why delay 1.0 for 0.12.4? 0.12-stable is fully merged into trunk, so I don't see what this would get us. And #10569 doesn't strike me as essential either (i.e. we shipped 0.12 without that, and it worked fine).

We should really stop worrying about releases having to be perfect, and just ship (and more often). But I repeat myself.

[Christian Boos]

Replying to rblank:

The TestPreferences also seems to point to a session storage failure, because the symptom is that the "Your preferences have been saved." message shown after saving preferences persists after going to a different pref page.

… it persists because the removal of the notification from the session has not been saved yet! We used to always save the session before doing the send(), but thanks to my wonderful suggestion in ticket:9453#comment:1, we now do the save after the send(), which makes it possible to have inconsistencies like that.

[Remy Blank]

Session saving was fixed in [11236].

[Christian Boos]

A few issues with svn 1.7.6 detected (the sort order of entries from DiffChangeEditor.deltas() changed)…

Time to sort those entries by ourselves (the order so far was meaningless anyway).

[Remy Blank]

Fixed ordering in [11237].

[Remy Blank]

I have found one issue that I would like to try and fix. In the repository index, all repositories always appear, even those that a user doesn't have access to. We should perform a permission check on the repository list, and only show the repositories that a user can access (using Repository.is_viewable()). I have a partial patch, but need to test it more thoroughly.

[Remy Blank]

The permission issue is caused by the way we allow access to parent directories of allowed files and directories, which is currently incorrect if a user is denied access in a repository-specific section, but is allowed in the "generic section". Here's a patch for the unit tests that demonstrates the issue:

trac/versioncontrol/tests/svn_authz.py

@@ -184 +184 @@
 [/precedence_d]
 @group1 = r
 user =
+[module:/secret]
+spy =
+[/secret]
+spy = r
 
 # Aliases
 [/aliases_a]
@@ -348 +352 @@
     def test_precedence(self):
         # Module-specific sections take precedence over non-module sections
         self.assertPathPerm(False, 'user', 'module', '/precedence_a')
+        self.assertPathPerm(False, 'spy', 'module', '/secret')
+        # ... even for parent directories
+        self.assertPathPerm(False, 'spy', 'module', '/')

The last assertion above fails. It's tricky to fix, and I haven't been able to come up with a good solution yet. I'll try again tomorrow.

[Christian Boos]

Replying to rblank:

[…] currently incorrect if a user is denied access in a repository-specific section, but is allowed in the "generic section".

+[module:/secret]
+spy =
+[/secret]
+spy = r
...
+        self.assertPathPerm(False, 'spy', 'module', '/')

The last assertion above fails. It's tricky to fix, and I haven't been able to come up with a good solution yet. I'll try again tomorrow.

While I see the problem, I don't think we absolutely need to address it before 1.0 (no one noticed this issue in 0.12.x). Even getting the patch "just in time" could be a bit problematic as it might have a non-negligible performance impact… So IOW, no hurries, we can make a 1.0.1 ticket for that and proceed with the release as scheduled this evening.

[Remy Blank]

Replying to cboos:

While I see the problem, I don't think we absolutely need to address it before 1.0 (no one noticed this issue in 0.12.x). Even getting the patch "just in time" could be a bit problematic as it might have a non-negligible performance impact…

I would almost agree, except for the fact that this is a security issue. It leaks information that is expected to be hidden. In the meantime I also found out that you could get the whole changelog (i.e. changeset times, authors and messages) of a repository to which you don't have access, under certain conditions (I haven't figured out the exact conditions yet).

(I'm not worried about a possible performance impact, the ideas I currently have would actually make the check faster.)

If we choose not to include this in 1.0, prepare yourself for a strong push from my side to release 1.0.1 very, very soon :)

[Christian Boos]

Replying to rblank:

(I'm not worried about a possible performance impact, the ideas I currently have would actually make the check faster.)

Ok, if you're confident about that, I'm less reluctant to wait for the fix.

If we choose not to include this in 1.0, prepare yourself for a strong push from my side to release 1.0.1 very, very soon :)

It's okay… people would be surprised (shocked?) anyway if we would strictly stick to our schedule ;-)

[Remy Blank]

Replying to cboos:

Ok, if you're confident about that, I'm less reluctant to wait for the fix.

Well, that part was only about performance. The thing is, I'm not sure that I'll actually find a sufficiently good solution in a reasonable time, or even that there is a solution at all (the part where we authorize access to directories if at least one sub-directory or file is authorized is currently a heuristic based only on the authz file; to be strictly correct, we would need to use the complete tree of the repository, which would be prohibitively expensive).

So if your intention was to release 1.0 this week-end, I'm ok with it. As you said, no one noticed in 0.12, and we can still release 1.0.1 when we have a good fix (along with other possible improvements).

[Christian Boos]

My intention was to release it now, but I see that Itamar is racing to get the translation fully complete, so I'll wait for him ;-)

[Christian Boos]

(In the meantime I'm releasing 0.12.4 as a warm-up exercise…)

[Itamar Oren]

Replying to cboos:

My intention was to release it now, but I see that Itamar is racing to get the translation fully complete, so I'll wait for him ;-)

Wow, thanks for that! :-) I'm not quite finished, but I think I'll stop now, so don't hold up because of me.

[Itamar Oren]

Replying to cboos:

(In the meantime I'm releasing 0.12.4 as a warm-up exercise…)

Nitpicking: Looks like you were off-by-7-months in the 0.12.4 ChangeLog​…

[Christian Boos]

Replying to itamaro:

.. you were off-by-7-months in the 0.12.4 ChangeLog​…

Ah, hm, yes … will fix (but well, of course that will stay for 0.12.4 itself).

[Christian Boos]

We did it ;-) Closing.

We'll do the announcements later tomorrow, once we get confirmation that everything is fine with the packages.

If there's any issue with the following set of packages, please reopen.

• e07f052ec91efc1733cb9d49743ecb32 Trac-1.0.tar.gz
• c6196ed293a992865182f4301a7b9efe Trac-1.0.win32.exe
• 3dd2e883179879c8fa5b98c984f15058 Trac-1.0.zip

(see TracDownload#LatestStableRelease for the actual links)

[Thijs Triemstra]

Awesome! Note that trunk/RELEASE still refers to 1.0rc1.

[Christian Boos]

Replying to thijstriemstra:

Awesome! Note that trunk/RELEASE still refers to 1.0rc1.

Oh, good catch. I grep'ed for rc1, but probably only in .py files… Fixed in r11317.