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 -- Michal Čihař | http://cihar.com | http://blog.cihar.com
On Tue, Feb 9, 2016 at 12:17 PM, Michal Čihař <michal@cihar.com> wrote:
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
-- Michal Čihař | http://cihar.com | http://blog.cihar.com
_______________________________________________ Developers mailing list Developers@phpmyadmin.net https://lists.phpmyadmin.net/mailman/listinfo/developers
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):
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.
Okay, let's process with this (see mail I've sent you privately).
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 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... -- Michal Čihař | http://cihar.com | http://blog.cihar.com
Hi, On 2/10/16 2:40 AM, Michal Čihař wrote:
Hi
Dne 9.2.2016 v 21:25 Dan Ungureanu napsal(a):
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.
Okay, let's process with this (see mail I've sent you privately).
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 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
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:
Hi,
On 2/10/16 2:40 AM, Michal Čihař wrote:
Hi
Dne 9.2.2016 v 21:25 Dan Ungureanu napsal(a):
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.
Okay, let's process with this (see mail I've sent you privately).
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 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
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.
_______________________________________________ Developers mailing list Developers@phpmyadmin.net https://lists.phpmyadmin.net/mailman/listinfo/developers
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). -- Michal Čihař | http://cihar.com | http://blog.cihar.com
Le 11/02/2016 08:14, Michal Čihař a écrit :
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).
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.
_______________________________________________ Developers mailing list Developers@phpmyadmin.net https://lists.phpmyadmin.net/mailman/listinfo/developers
Hi Dne 13.2.2016 v 10:17 Remi Collet napsal(a):
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.
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 -- Michal Čihař | http://cihar.com | http://blog.cihar.com
Hi Dne 15.2.2016 v 08:44 Michal Čihař 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:
And pull request is here: https://github.com/phpmyadmin/phpmyadmin/pull/11976 Comments welcome :-). -- Michal Čihař | http://cihar.com | http://blog.cihar.com
On Thu, Feb 18, 2016 at 6:54 PM, Michal Čihař <michal@cihar.com> wrote:
Hi
Dne 15.2.2016 v 08:44 Michal Čihař 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:
And pull request is here:
https://github.com/phpmyadmin/phpmyadmin/pull/11976
Comments welcome :-).
-- Michal Čihař | http://cihar.com | http://blog.cihar.com
_______________________________________________ Developers mailing list Developers@phpmyadmin.net https://lists.phpmyadmin.net/mailman/listinfo/developers
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):
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 haven't seen this problem, but 804a091e5a862fc174476421969cda85ff1af652 should address it. -- Michal Čihař | http://cihar.com | http://blog.cihar.com
participants (4)
-
Dan Ungureanu -
Isaac Bennetch -
Michal Čihař -
Remi Collet