Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rain variables #11

Merged
merged 4 commits into from
Aug 12, 2014
Merged

Rain variables #11

merged 4 commits into from
Aug 12, 2014

Conversation

Sbgodin
Copy link

@Sbgodin Sbgodin commented Jul 31, 2014

Hi all!

I wish to add the ability to define where to store all writable files. I set up two variables for RainTPL. This way, it will be possible to install/setup Shaarli in two separate directory : one with write access, one with read only access.

I think it would be neat for package maintainer and, maybe, to have a multi-user deployment.

Would you like to check this, at least, the following ways:

  • Is the idea valuable?
  • Is there any error in it?
  • Do you like my implementation?

Thanks in advance.

Sbgodin added 2 commits July 31, 2014 23:31
The path for templates and temporary files are now part of the configuration.

For a custom install, it's possible to put these writable directories elsewhere than in the read-only source code.
The same test is already on line 93
@e2jk
Copy link

e2jk commented Aug 1, 2014

I'm not sure I see the value in the variable for the template files, as those don't need write access.

But I second the idea of write vs. read-only access, as that is what the Debian package already performs:
The "static" files (index.php, .html template files) are installed in /usr/share/shaarli, while the "dynamic" data is stored at other places:

  • data to /var/lib/shaarli/data
  • cache to /var/cache/shaarli/cache
  • pagecache to /var/cache/shaarli/pagecache
  • raintpl temp cache to /var/cache/shaarli/tmp

In the Debian package, the paths are hardcoded (in debian/patches/install.patch, but it would be as fine to have them configurable using the config file, and Debian would provide a standard config file with these paths already set.

For completeness' sake, FYI there is a second patch debian/patches/raintpl-tmp-folder-creation.patch that uses the raintpl::$cache_dir variable instead of tmp. Seems like I hadn't noticed the duplication of that piece of code ;)

Could you increase the scope of this pull request, to also allow indicating the storage location for data, cache and pagecache? This would allow me to drop 2 patches from the package, which is great news.

@nodiscc
Copy link
Member

nodiscc commented Aug 1, 2014

Hey nice patch, I would also remove the chmod/mkdir calls from index.php, this way we can remove the need for shaarli to be able to write in it's own directory. (this has been reported sebsauvage#181).

Just create empty tmp/ and data directories in the repo with correct write permissions, so that the default setting works. (since git does not track empty directories, create a blank .gitignore in these dirs)

@Sbgodin
Copy link
Author

Sbgodin commented Aug 1, 2014

I grepped all the mkdir statements, it's about : DATADIR, RAINTPL_TMP, CACHEDIR, PAGECACHE. Special mention about the french provider free.fr and the session directory to create.

@nodiscc If I follow your thoughts correctly, we would create in the repository the following directories : data, tmp, cache, pagecache. This way, we could get rid of the mkdir. Is this ok?

I added the template directory, just to be able to customize all the paths. Let's imagine that, later, templates are added. An option could allow to choose one or another, especially if Shaarli go mutli-users...

@e2jk the location of the cache, pagecache, and data were already existing as variables. As you can see on line 37 you can add DATADIR/options.php to override the values.

@nodiscc
Copy link
Member

nodiscc commented Aug 1, 2014

we would create in the repository the following directories : data, tmp, cache, pagecache. This way, we could get rid of the mkdir. Is this ok?

Yes that's what I had in mind.

just to be able to customize all the paths. Let's imagine that, later, templates are added. An option could allow to choose one or another

Very good. Same could be said for custom themes/CSS as we have already collected a few

@nodiscc nodiscc mentioned this pull request Aug 3, 2014
@Sbgodin Sbgodin self-assigned this Aug 3, 2014
Sbgodin added 2 commits August 4, 2014 00:41
They are still in .gitignore because their future content will still be ignored.
I also removed the previously created placeholders, which after all, have no more utility.
@Sbgodin
Copy link
Author

Sbgodin commented Aug 3, 2014

I added the empty directories, filling them with an htaccess denying access. I also removed the directories creation and the creation of the htaccess.

@Sbgodin
Copy link
Author

Sbgodin commented Aug 11, 2014

I didn't see any problem with it. Is it ok for you that I merge it in master?

@nodiscc
Copy link
Member

nodiscc commented Aug 12, 2014

Let's do this. Thanks!

nodiscc added a commit that referenced this pull request Aug 12, 2014
@nodiscc nodiscc merged commit 1ec633a into shaarli:master Aug 12, 2014
@Sbgodin Sbgodin deleted the rainVariables branch August 19, 2014 19:35
ajabep added a commit to ajabep/Shaarli that referenced this pull request Apr 2, 2021
When we try to access the atom feed and have no bookmarks, it raised the following exception :

```
Call to a member function reorder() on array /webroot/application/bookmark/BookmarkFileService.php:143
#0 /webroot/application/feed/FeedBuilder.php(106): Shaarli\Bookmark\BookmarkFileService->search(Array, 'public', false, false, true)
shaarli#1 /webroot/application/front/controller/visitor/FeedController.php(47): Shaarli\Feed\FeedBuilder->buildData('atom', Array)
shaarli#2 /webroot/application/front/controller/visitor/FeedController.php(20): Shaarli\Front\Controller\Visitor\FeedController->processRequest('atom', Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#3 [internal function]: Shaarli\Front\Controller\Visitor\FeedController->atom(Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#4 /webroot/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php(40): call_user_func(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#5 /webroot/vendor/slim/slim/Slim/Route.php(281): Slim\Handlers\Strategies\RequestResponse->__invoke(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#6 /webroot/application/front/ShaarliMiddleware.php(55): Slim\Route->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#7 [internal function]: Shaarli\Front\ShaarliMiddleware->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#8 /webroot/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Array, Array)
shaarli#9 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#10 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#11 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\Route->Slim\{closure}(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#12 /webroot/vendor/slim/slim/Slim/Route.php(268): Slim\Route->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#13 /webroot/vendor/slim/slim/Slim/App.php(503): Slim\Route->run(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#14 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\App->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#15 /webroot/vendor/slim/slim/Slim/App.php(392): Slim\App->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#16 /webroot/vendor/slim/slim/Slim/App.php(297): Slim\App->process(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#17 /webroot/index.php(177): Slim\App->run(true)
shaarli#18 {main}
```
ArthurHoaro pushed a commit to ArthurHoaro/Shaarli that referenced this pull request May 8, 2021
When we try to access the atom feed and have no bookmarks, it raised the following exception :

```
Call to a member function reorder() on array /webroot/application/bookmark/BookmarkFileService.php:143
#0 /webroot/application/feed/FeedBuilder.php(106): Shaarli\Bookmark\BookmarkFileService->search(Array, 'public', false, false, true)
#1 /webroot/application/front/controller/visitor/FeedController.php(47): Shaarli\Feed\FeedBuilder->buildData('atom', Array)
#2 /webroot/application/front/controller/visitor/FeedController.php(20): Shaarli\Front\Controller\Visitor\FeedController->processRequest('atom', Object(Slim\Http\Request), Object(Slim\Http\Response))
#3 [internal function]: Shaarli\Front\Controller\Visitor\FeedController->atom(Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
#4 /webroot/vendor/slim/slim/Slim/Handlers/Strategies/RequestResponse.php(40): call_user_func(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
#5 /webroot/vendor/slim/slim/Slim/Route.php(281): Slim\Handlers\Strategies\RequestResponse->__invoke(Array, Object(Slim\Http\Request), Object(Slim\Http\Response), Array)
shaarli#6 /webroot/application/front/ShaarliMiddleware.php(55): Slim\Route->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#7 [internal function]: Shaarli\Front\ShaarliMiddleware->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#8 /webroot/vendor/slim/slim/Slim/DeferredCallable.php(57): call_user_func_array(Array, Array)
shaarli#9 [internal function]: Slim\DeferredCallable->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#10 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(70): call_user_func(Object(Slim\DeferredCallable), Object(Slim\Http\Request), Object(Slim\Http\Response), Object(Slim\Route))
shaarli#11 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\Route->Slim\{closure}(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#12 /webroot/vendor/slim/slim/Slim/Route.php(268): Slim\Route->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#13 /webroot/vendor/slim/slim/Slim/App.php(503): Slim\Route->run(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#14 /webroot/vendor/slim/slim/Slim/MiddlewareAwareTrait.php(117): Slim\App->__invoke(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#15 /webroot/vendor/slim/slim/Slim/App.php(392): Slim\App->callMiddlewareStack(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#16 /webroot/vendor/slim/slim/Slim/App.php(297): Slim\App->process(Object(Slim\Http\Request), Object(Slim\Http\Response))
shaarli#17 /webroot/index.php(177): Slim\App->run(true)
shaarli#18 {main}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants