Bug in tree view directory ?

[ogou]

I'm using websvn with the tree view and Calm template.
It seem to have a bug with the Path/Tree view directory presentation.
I can't see path directory in the tree view (can just see the files).

Version:
PHP 7.2.24
Apache/2.4.37

[michael-o]

Both versions you use are already out of support. Please upgrade first.

[ogou]

I've update to PHP 7.4, but same issue...
I can't find the supported version for Websvn.

[ogou]

I've just test with PHP 7.x and PHP 8.x and it seem to be the same.

[michael-o]

That's not a bug, I'd say. This is a deliberate change.
Either f22a46b or 16168ec

[ogou]

Thank's for you comment !
But just to be clear, it is not possible to have the sub-directory tree view with this new version ?

[michael-o]

Please read the commit message. As far as I understood @ams-tschoening it might lead to errors if permissions are restrictive. Maybe this can be turned into a config option to provide old behavior, if required. WDYT?

[ams-tschoening]

Please provide the concrete URLs you are opening and observing the wrong rendering. Look at the linked PR, go to that code base and simply change $level back to 0 like it was before and see what happens. If this fixes it for you, it seems there needs to be some config, though I don't understand why @k10blogger and me didn't recognize such problem as well already.

OTOH, there were rendering changes regarding open closed files and directories in the past. Maybe your problem is related to those instead:

#122

[ogou]

I've just change the $level to 0.
This modification solve my problem ;-)

So yes, is it possible to add an option in the conf file to manage this ?

[k10blogger]

Can you attach a snapshot how it looks after changing $level = 0

[ogou]

[ams-tschoening]

There is a wider underlying problem here: What to render if NO restrictions are in place vs. when those ARE in place and people have access to subdirs only. Having a config, one can only switch between using my different implementation and not, while there can easily be different repos with and without access restrictions published by the same WebSVN. So in theory that config would need to per repo and even that wouldn't work for people who simply have access to all subdirs within the repo.

From my understanding, the correct implementation would start at the root of the repo and check if that is readable or not. If so, things are fine and contents can be rendered. If not, one would need to decide what to render instead, because we have two cases: either the user is not allowed to see anything of the requested path or only some subpaths. What to render heavily depends on those results and the former implementation simply stopped at the root without access. That is wrong as well as is not rendering the parents if those are visible, like seems to be the case for the reporter of this issue.

Though, I wonder why that doesn't happen already to other users? Starting to check/render with a subdir should happen pretty often and the current implementation doesn't check if auth restrictions are in place or stuff like that. That's the whole problem...

Anyway, maybe my PR should simply be reverted for now to have the old behavior? My use case might be pretty special and I'm somewhat sure to abandon WebSVN for that use case in future anyway most likely. Handling these cases correctly seems to be pretty difficult, so the old wrong behaviour working for most people might be the easiest way to go.

[michael-o]

The current behavior is beneficial for the performance. Regardless of this, the option is even per-user/-group because it well be different for both. I think we should resemble the same behavior mod_dav_svn applies when you use a browser. A per repo/global option would be nice and at the descretion of the admin.

[ams-tschoening]

I think we should resemble the same behavior mod_dav_svn applies when you use a browser.

And how does that look like?

@ogou Do you have any access restrictions for your repos in place?

[michael-o]

I think we should resemble the same behavior mod_dav_svn applies when you use a browser.

And how does that look like?

I can't exactly tell, need to look into source, but the output is always level aware. It does show only the current dir context.

[ogou]

@ogou Do you have any access restrictions for your repos in place?

No restriction in my configuration.
All users have access to all svn projects, and no restriction in the repo
A simple configuration.

[k10blogger]

The SVN output should always show the current path or current directory. I think even visual SVN repo browser does the same. It focuses only on the current directory.
We should not bring $level back to 0 as for bigger repositories it is pain with little to no gain.
If you want tree and sub tree configuraiton and have no restriction in the repo use the following in the config
$config->setLoadAllRepos(true);
This is one time load pain for very big repositories but after that it is smooth and you get the view you want.

[ams-tschoening]

The parent names of the requested directory could as well be rendered always without any content and without any access checks. Really only print the directory names. That would show what @ogou misses now, use cases with access restrictions in subdirs would keep working and performance would stay the same as currently. Something like:

showDirs($svnrep, $subs, 0, $level, $rev, $peg, $listing, 0, $config->treeView);

OTOH, the output would be different from the past. Without access checks, one couldn't provide the last log message and stuff.

@ogou Please try $config->setLoadAllRepos(true); as suggested. Is that fast enough for you?

[ogou]

@ogou Please try $config->setLoadAllRepos(true); as suggested. Is that fast enough for you?

I've try to add this parameter into the config.php file.
For a small svn project, it's ok
But for big svn project, it's keep loading for 10/20 seconds (slow), and then it crashed with an http error 500
-> in the logs file, fatal error for allocation memory ! (I can try to modify this php parameter, but I think that it would be too slow)

So for me, the best solution would be this modification -> $level = 0

[ams-tschoening]

So for me, the best solution would be this modification -> $level = 0

So for the time being, the easiest solution is you fork the repo, change that code yourself and maintain your own fork. However things will be changed here, when updating, GIT will easily tell you about possible conflicts and you can decide what to do.

The other easiest option would be reverting my changes and decide later how to deal with those. As said, I don't care too much anymore and that might not be worth breaking other users with their use-cases.

[michael-o]

So for me, the best solution would be this modification -> $level = 0

So for the time being, the easiest solution is you fork the repo, change that code yourself and maintain your own fork. However things will be changed here, when updating, GIT will easily tell you about possible conflicts and you can decide what to do.

The other easiest option would be reverting my changes and decide later how to deal with those. As said, I don't care too much anymore and that might not be worth breaking other users with their use-cases.

I would accept an option. Have all the tree costs too much performance and doens't really make sense. I'd object a pure revert.

[k10blogger]

I too object on a pure revert.
But lets think on this. There could be some middle ground.
For time being we can add an option. That doesnt harm as far as I can see.(Though would be weird) If users want to see they can enable the option otherwise it would be disable by default.

[MosfetN]

it possible to add an option in the conf

Hi @ogou,
Thanks for your investigations, I have the same issue upgrading from 2.3.0 to 2.7.0.
Would please tell me where you did change $level = 0 ?

[ogou]

Hi @ogou, Thanks for your investigations, I have the same issue upgrading from 2.3.0 to 2.7.0. Would please tell me where you did change $level = 0 ?

No option in the conf file right now.
So you must change the code into the file listing.php (function showTreeDir)
Line 445 :
$limit = count($subs) - 2;
$level = $limit;
$level = $level <= 0 ? 0 : $level;

And add $level = 0;

[michael-o]

Hi @ogou, Thanks for your investigations, I have the same issue upgrading from 2.3.0 to 2.7.0. Would please tell me where you did change $level = 0 ?

No option in the conf file right now. So you must change the code into the file listing.php (function showTreeDir) Line 445 : $limit = count($subs) - 2; $level = $limit; $level = $level <= 0 ? 0 : $level;

And add $level = 0;

Do you want to provide a patch for this config option?