| Message ID | 20221205214838.27594-1-yarl-baudig@mailoo.org |
|---|---|
| Headers | show |
| Series | Add PMB (Integrated Library System) | expand |
I'm sending a version 2 to clean lint errors (badly configured emacs) + fix some random typo in the manual patch. I hope this will pass the qa test.
Thank you for all your comments. I will rework this. I have some questions if you will. I know there is a problem with this package/service. I feel I am sort of breaking the functional paradigm. I also feel the package is "leaking" into the service, because the package is completely useful by itself, I'm not even talking about web server, PHP or MYSQL. The package is only a 'source' for a service. If that makes sense. > > > + ;; Those are to hide installation files on a re-activation > > + (if > > + (file-exists? (string-append #$http-directory > "/tables/noinstall.php")) > > + (delete-file (string-append #$http-directory > "/tables/install.php"))) > > + (if > > + (file-exists? (string-append #$http-directory > "/tables/noinstall_rep.php")) > > + (delete-file (string-append #$http-directory > "/tables/install_rep.php")))))) > > Or just: > > (false-if-exception (delete-file …)) > That's not the same thing (check noinstall to remove install). Indeed, after the first service installation, user interaction is required, as explained in the manual : set up of database password then (and that's the problematic part) initialization/configuration of pmb through a web browser by running "install.php". So, you need install.php. One time. This initialization takes care of (at least) moving install.php to noinstall.php. Same logic for install_rep.php. This is also the why of the snippet below (the copy), because the application need to be in a read-write directory. The purpose of the code above is only to differentiate between an installation and an upgrade. The service keeps state of itself in http-directory, already installed or not, here I break the functional paradigm? Or that's not so bad as it's not a package but a service? > > + #~(begin > > + (use-modules (guix build utils)) > > + (use-modules (ice-9 ftw)) > > + (format #t "Copying pmb '~a' to http directory '~a'~%" > > + (string-append #$pmb "/share/http/pmb") > > + #$http-directory) > > + (mkdir-p #$http-directory) > > + ;; Please be quiet > > + (call-with-output-file "/dev/null" > > + (lambda (port) > > + (with-output-to-port port > > + (lambda () > > + (copy-recursively (string-append #$pmb > "/share/http/pmb") > > + #$http-directory))))) > > Do we really need to copy these files? > > Keep in mind that activation snippets run each time you boot the system, > and each time you reconfigure. It’s best to avoid doing too much work > there. > Now, I see one solution to avoid the copy (and the chown), apart from the very first activation. That's keeping even more state into http-directory : for example a file ".version" with the hash of the pmb package used inside. That way we know if there's really an upgrade. What do you think?
Hi, (Cc: Julien who’s worked on PHP packaging before.) "yarl baudig" <yarl-baudig@mailoo.org> skribis: > I know there is a problem with this package/service. I feel I am sort of breaking the functional paradigm. I also feel the package is "leaking" into the service, because the package is completely useful by itself, I'm not even talking about web server, PHP or MYSQL. The package is only a 'source' for a service. If that makes sense. Hmm, I see what you mean. But maybe the same can be said of many (most?) PHP packages? >> > + ;; Those are to hide installation files on a re-activation >> > + (if >> > + (file-exists? (string-append #$http-directory >> "/tables/noinstall.php")) >> > + (delete-file (string-append #$http-directory >> "/tables/install.php"))) >> > + (if >> > + (file-exists? (string-append #$http-directory >> "/tables/noinstall_rep.php")) >> > + (delete-file (string-append #$http-directory >> "/tables/install_rep.php")))))) >> >> Or just: >> >> (false-if-exception (delete-file …)) >> > > That's not the same thing (check noinstall to remove install). Oops, my bad, sorry. > Indeed, after the first service installation, user interaction is > required, as explained in the manual : set up of database password > then (and that's the problematic part) initialization/configuration of > pmb through a web browser by running "install.php". So, you need > install.php. One time. This initialization takes care of (at least) > moving install.php to noinstall.php. Same logic for install_rep.php. > This is also the why of the snippet below (the copy), because the > application need to be in a read-write directory. The purpose of the > code above is only to differentiate between an installation and an > upgrade. The service keeps state of itself in http-directory, already > installed or not, here I break the functional paradigm? Or that's not > so bad as it's not a package but a service? That’s quite unusual, it would be nice for the comment to explain this. But it’s okay to do it this way; don’t fear the paradigm, it’s here to serve us, not the other way around. :-) >> > + #~(begin >> > + (use-modules (guix build utils)) >> > + (use-modules (ice-9 ftw)) >> > + (format #t "Copying pmb '~a' to http directory '~a'~%" >> > + (string-append #$pmb "/share/http/pmb") >> > + #$http-directory) >> > + (mkdir-p #$http-directory) >> > + ;; Please be quiet >> > + (call-with-output-file "/dev/null" >> > + (lambda (port) >> > + (with-output-to-port port >> > + (lambda () >> > + (copy-recursively (string-append #$pmb >> "/share/http/pmb") >> > + #$http-directory))))) >> >> Do we really need to copy these files? >> >> Keep in mind that activation snippets run each time you boot the system, >> and each time you reconfigure. It’s best to avoid doing too much work >> there. >> > > Now, I see one solution to avoid the copy (and the chown), apart from the very first activation. That's keeping even more state into http-directory : for example a file ".version" with the hash of the pmb package used inside. That way we know if there's really an upgrade. What do you think? It sounds like it’s going a bit far in terms of complexity. Why do we need to copy these files every time? Do they actually have to be writable? If not, whatever this copying step is doing could be done in a ‘computed-file’. Does that make sense? Thanks, Ludo’.
> > > > Now, I see one solution to avoid the copy (and the chown), apart from the > very first activation. That's keeping even more state into http-directory : > for example a file ".version" with the hash of the pmb package used inside. > That way we know if there's really an upgrade. What do you think? > > It sounds like it’s going a bit far in terms of complexity. > > Why do we need to copy these files every time? Do they actually have to > be writable? If not, whatever this copying step is doing could be done > in a ‘computed-file’. > > Does that make sense? > Hmm no, not really. Yes it has to be writable for the interactive initialization through web browser to take place. At least two operations take place there that need RW : 1/ renaming of both install.php and install_req.php. 2/ saving into a file the url and credentials for connexion to the database. There is no _need_ to copy every time. I did like this in my first patchs because it was _correct_ (I didn't think about efficiency then). That's why I am proposing the solution which copy only for the very fist activation and for the first activation after a package upgrade (or downgrade). I don't see how we could avoid this. I don't think it's a good idea to try to avoid the interactive installation as it gives informations and options on the installation to the user. Why "computed-file"?
Hi, "yarl baudig" <yarl-baudig@mailoo.org> skribis: >> > >> > Now, I see one solution to avoid the copy (and the chown), apart from the >> very first activation. That's keeping even more state into http-directory : >> for example a file ".version" with the hash of the pmb package used inside. >> That way we know if there's really an upgrade. What do you think? >> >> It sounds like it’s going a bit far in terms of complexity. >> >> Why do we need to copy these files every time? Do they actually have to >> be writable? If not, whatever this copying step is doing could be done >> in a ‘computed-file’. >> >> Does that make sense? >> > > Hmm no, not really. Yes it has to be writable for the interactive initialization through web browser to take place. At least two operations take place there that need RW : > 1/ renaming of both install.php and install_req.php. > 2/ saving into a file the url and credentials for connexion to the database. Credentials have to be handled out-of-band anyway (keep in mind that /gnu/store is world-readable so in general we cannot have password in transit via the store). > There is no _need_ to copy every time. I did like this in my first patchs because it was _correct_ (I didn't think about efficiency then). > > That's why I am proposing the solution which copy only for the very fist activation and for the first activation after a package upgrade (or downgrade). > > I don't see how we could avoid this. I don't think it's a good idea to try to avoid the interactive installation as it gives informations and options on the installation to the user. OK. > Why "computed-file"? Take it with a grain of salt because I still have an incomplete understanding of the initialization process of PMB. My thought was that, if there’s a “computation” (copying files around, renaming some) that can be done once and for all, then we could do that in ‘computed-file’. But what you’re saying, IIUC, is that this is not done “once and for all”. HTH, Ludo’.