Hi all
current handlig of SQL parser code is less than optimal - it's developed in external repository and at "random" times copied to phpMyAdmin. That makes it quite hard to maintain - the fixes made in phpMyAdmin repository will be probably lost and it's hard to track what version went it.
Therefore I suggest:
- to move sql-parser git repo under phpmyadmin - to use sql-parser as submodule in phpmyadmin
This way, all development of SQL parser will be in single Git repository where all phpMyAdmin developers have access.
There are alternative approaches to handle this, but I find this best fitting our current model (no external dependencies).
[1]:https://github.com/udan11/sql-parser
On Tue, Feb 9, 2016 at 12:17 PM, Michal Čihař michal@cihar.com wrote:
Hello,
The first time I sent my proposal for Google Summer of Code 2015 I tried finding libraries that would fit phpMyAdmin. Because I found none I thought that I could create a library on my own hoping that others (not only phpMyAdmin) would benefit from it and will help developing it (testing, sending bug reports, etc). I talked with Marc and he seemed to be fine with this.
After the summer ended, I was planning on talking with Marc to transfer the library to phpMyAdmin, but I did not get to do that. Hopefully, we can do this soon.
In my opinion, the best option would be to manage dependencies with Composer, but I proposed this in the past and people did not consider this a very good idea. Anyway, I still believe that keeping the library in a different repository is the best way because: - it makes bug fixing easier; no need to merge upstream, older versions of phpMyAdmin may be easier to patch, etc; - the changes to the library don't pollute the change log of phpMyAdmin; - it is designed as a "module" to phpMyAdmin and the logic related to SQL queries is separated; - maybe at some time in the future the library will also be used in other projects.
I am looking forward for more suggestions. During the past couple of weeks I have been quite inactive, but I am done with exams for now and in the next days I am hoping to send fixes prepared for most issues that I assigned to myself.
Best regards, Dan Ungureanu
Hi
Dne 9.2.2016 v 21:25 Dan Ungureanu napsal(a):
Okay, let's process with this (see mail I've sent you privately).
I still want the library to be kept in separate repository. I just want to embed that one inside phpmyadmin one as a submodule, see https://git-scm.com/docs/git-submodule
I'm not sure about using Composer - people still expect to find complete tarball on our website...
Hi,
On 2/10/16 2:40 AM, Michal Čihař wrote:
On the surface, this seems like quite a good solution, but from what I recall (and a quick look at that page confirms my memory), it is a bit awkward to work with submodules. For instance, one has to manually update/sync to the submodule and it seems easy to get out of sync. I've never worked with submodules and I definitely might be wrong about this, but the documentation makes this process sound awkward.
I'm not sure about using Composer - people still expect to find complete tarball on our website...
I agree that we should distribute the complete tarball and not require using Composer for this.
On Wed, Feb 10, 2016 at 3:21 PM, Isaac Bennetch bennetch@gmail.com wrote:
Hello,
As far as I know, submodules must be pulled manually and this might confuse people.
The script that generates the release can be modified to install Composer dependencies first and then create the tarball.
Best regards, Dan Ungureanu
Hi
Dne 10.2.2016 v 18:00 Dan Ungureanu napsal(a):
As far as I know, submodules must be pulled manually and this might confuse people.
Indeed that's true, but that's true for Composer as well :-).
The script that generates the release can be modified to install Composer dependencies first and then create the tarball.
That's an option as well. Looking at the external libraries we use, some of them are available in packagist as well (tcpdf, phpseclib).
Le 11/02/2016 08:14, Michal Čihař a écrit :
One important thing for downstream distribution of phpMyAdmin is to be able to easily track such external dependencies.
At least having this information (in changelog) seems useful for us
ex: - update udan11/sql-parser to 3.1.0
(IIRC, at some point I have missed the switch from phpseclib v1 to v2)
Composer.json also is a nice way to have good description of dependency
requires: { "udan11/sql-parser": "^3.1", }
And composer.lock in the release tag gives even more information (exact version, commit reference, for each bundled external lib).
Having a "vendor" dir with all the external lib is rather a good thing, and easy to manage downstream, just have to remove its content and create an vendor/autoload.php to use the system libraries instead.
This can also allow to cleanup the libraries/vendor_config.php file.
Of course, the "vendor" dir have to be included in the distributed tarball, but doesn't have to be in the git repo, developer are probably used to run "composer install" in such project.
Remi.
Hi
Dne 13.2.2016 v 10:17 Remi Collet napsal(a):
Thinking more about this, using composer is probably best approach here. It's too late for including this change in 4.6.0, but we can target on 4.7.0:
https://github.com/phpmyadmin/phpmyadmin/issues/11962
Hi
Dne 15.2.2016 v 08:44 Michal Čihař napsal(a):
And pull request is here:
https://github.com/phpmyadmin/phpmyadmin/pull/11976
Comments welcome :-).
On Thu, Feb 18, 2016 at 6:54 PM, Michal Čihař michal@cihar.com wrote:
Hello Michael,
The pull request seems fine, but I did not have really much time to check it properly (I'm out of town and had little time to properly check it out). However, I have one question: the old class autoloader that was used to include the sql-parser required php-gettext before registering [1] the autoloader because of issue #11677 [2]. Is there any mechanism to include php-gettext first so this problem does not show up again?
I hope I will have more free time to properly review and test it.
[1] https://github.com/phpmyadmin/phpmyadmin/blob/master/libraries/sql-parser/au... [2] https://github.com/phpmyadmin/phpmyadmin/issues/11677
Best regards, Dan Ungureanu
Hi
Dne 18.2.2016 v 19:06 Dan Ungureanu napsal(a):
I haven't seen this problem, but 804a091e5a862fc174476421969cda85ff1af652 should address it.
-
Dan Ungureanu -
Isaac Bennetch -
Michal Čihař -
Remi Collet