Edgewall Software
Modify

Opened 14 years ago

Closed 12 years ago

#9585 closed enhancement (duplicate)

[PATCH] Irrelevant error on trac.log permission issue

Reported by: andre.miras@… Owned by:
Priority: low Milestone:
Component: version control Version: 0.12-stable
Severity: minor Keywords: git, post-receive, hook, permission, review
Cc: andre.miras@…, Thijs Triemstra Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Hi, I spent ages trying to understand what was behind a "Command not found" error while running the following command from the shell (for debugging git post-receive hooks):

trac-admin '/home/myuser/trac/project1/' 'changeset' 'added' '(default)' '2dd5db7daecab8a08a17de4f5e603d14db4125a0'

In my particular case, the expected output should had been:

IOError: [Errno 13] Permission denied: u'/var/log/trac.log'

This is because the exception raised by logger_handler_factory in trac/env.py(468)setup_log() is handled upper in trac/admin/console.py(151)env_check(). I think it should really be raised or handled in a more proper way so a trac administrator won't spend ages trying to figure out what is that wired "Command not found" message after his git push.

By prospecting on the web we can see that "Command not found" must be a permission which was right but irrelevant. As the user owning my TRAC_ENV (myuseer) is different as the user hosting git (git). I tried to give my git user a rwx access to my TRAC_ENV, after what I still got the issue.

And it's only after doing further investigation using pdb that I found out the permission issue came from my trac.log which stands in /var/log/trac.log After giving git a rw access to it, everything worked ok.

Additional informations: OS: Gentoo Linux Python 2.6.2 Trac 0.12 Trac git r8215 Debugging output attached

Attachments (3)

trac-debugging-output.txt (3.3 KB ) - added by andre.miras@… 14 years ago.
pdb output
env_check.patch (2.4 KB ) - added by Sebastian Krysmanski <sebastian@…> 13 years ago.
Patch against Trac 0.12.1
env_check.2.patch (2.4 KB ) - added by Sebastian Krysmanski <sebastian@…> 13 years ago.
Patch against Trac 0.12.1 (allows empty directories)

Download all attachments as: .zip

Change History (12)

by andre.miras@…, 14 years ago

Attachment: trac-debugging-output.txt added

pdb output

comment:1 by Remy Blank, 14 years ago

Milestone: next-minor-0.12.x
Owner: set to Remy Blank

Yes, that part of trac-admin could be improved. Another improvement would be to use the same open_environment() machinery as for web request processing, including reloading the environment when required (e.g. on configuration changes).

comment:2 by Remy Blank, 14 years ago

Milestone: next-minor-0.12.x0.13

The refactoring in comment:1 would be an enhancement for a major release.

by Sebastian Krysmanski <sebastian@…>, 13 years ago

Attachment: env_check.patch added

Patch against Trac 0.12.1

by Sebastian Krysmanski <sebastian@…>, 13 years ago

Attachment: env_check.2.patch added

Patch against Trac 0.12.1 (allows empty directories)

comment:3 by Sebastian Krysmanski <sebastian@…>, 13 years ago

I've included a patch that does exactly what the reporter want. It's not major rewrite, though, but this way this could get into the next minor release.

The patch itself is not very well arranged (but the result of the patch is), so I'm just losing some words on what I've changed:

The original implementation silently ignored all exceptions that were raised in the environment initialization propably because it should work when the environment hasn't been created yet. However this results in no clues whatsoever when something is wrong with the environment (especiallly permission problems) but makes trac-admin appear as if there were no environment.

So with my patch the only possibility for trac-admin to recognize an environment as "not existing" is if the directory doesn't exist or if it's empty. I'm not 100% sure but currently I can't think of any other situation in which a non-empty directory should be considered a "not existing" Trac environment.

Other than that there's only one API change in my patch. It regards env_set() where I've removed the env parameter (what's the user case for specifying an environment separated from its path?) and added a new parameter do_env_check which does the environment check directly inside env_set(). This way the user is immediately notified if something is wrong with the environment.

comment:4 by Remy Blank, 13 years ago

Milestone: 0.130.14

comment:5 by Thijs Triemstra, 13 years ago

Cc: Thijs Triemstra added

comment:6 by Thijs Triemstra, 13 years ago

Keywords: review added
Summary: Irrelevant error on trac.log permission issue[PATCH] Irrelevant error on trac.log permission issue

This patch needs a review.

comment:7 by Remy Blank, 12 years ago

Owner: Remy Blank removed

Refocusing.

comment:8 by Jun Omae, 12 years ago

Trac 0.12.3 doesn't have the issue, but 0.12.2 has the issue. I suppose this is fixed in [10856].

jun66j5@gotanda:774$ hg update -r trac-0.12.3
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
jun66j5@gotanda:775$ chmod a-w ~/var/trac/0.12-stable/log/trac.log
jun66j5@gotanda:776$ PYTHONPATH=$PWD python2.4 trac/admin/console.py ~/var/trac/0.12-stable repository list
Error: IOError: [Errno 13] Permission denied: u'/home/jun66j5/var/trac/0.12-stable/log/trac.log'

jun66j5@gotanda:777$ hg update -r trac-0.12.2
170 files updated, 0 files merged, 1 files removed, 0 files unresolved
jun66j5@gotanda:778$ make clean >/dev/null
jun66j5@gotanda:779$ PYTHONPATH=$PWD python2.4 trac/admin/console.py ~/var/trac/0.12-stable repository list
Error: Command not found

comment:9 by Remy Blank, 12 years ago

Milestone: 0.14
Resolution: duplicate
Status: newclosed

Indeed, so this was a duplicate of #10044 (or the other way around). Thanks for testing!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none) to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.