-
Notifications
You must be signed in to change notification settings - Fork 131
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
Import Latest Data fails with no error #259
Comments
I've tried to follow this through the code. When it gets to the following code, the $t_svn_cmd is good (I can copy it and paste in a command window and it runs ok) but the content of the $t_pipes array is empty
|
Is it correct that the second element of the third pipe is 'a'? Should it be 'w'? |
It actually used to be I don't use SVN or Windows myself, so I could not test this change before merging it, but looking at proc_open documentation, I see now that the descriptorspec definition only allows
Maybe 0c060a6 should be reverted. @FSD-Christian-ISS care to comment ? |
I've submitted pull request #261 to change it to |
Using "a" looks broken on PHP 7.2 running on Windows. Quick test script:
Running this yields:
Changing the 'a' to an 'w' & re-running provides the expected output. |
@bright-tools Thanks for your continued research on this.
Which is not surprising based on what the docs define, as mentioned in my earlier note #259 (comment) Are you implying that Anyway based on this discussion and the follow-up in @kabadi's pull request #261, I think the sensible thing to do is simply to revert 0c060a6. |
This reverts commit 0c060a6 (#254). As per discussion in #259, use of 'a' for pipe type is not documented in the PHP manual [1], and the proc_open() call does not work with it (STDERR returns no output). [1]: http://php.net/manual/en/function.proc-open.php
@dregad As you say, not surprising based on the docs. I was just doing a quick experiment to see what the actual behaviour was (seems to be that the stream contents is lost). I've not had chance to test it on any versions other than that which I mentioned. |
I ran a quick test over lunch break using WAMP server, I get the same results with 5.6 and 7.0. |
I'm not into this in detail myself, but a co-worker reported that he added something like this to get this issue fixed: In file SourceSVN.php, line 337 add if statement like this:
Maybe it helps... |
Thanks for the suggestion; it was one of the options which I looked at, but the PHP manual says of
which was enough to scare me off. |
Regarding the 'a' vs. 'w' discussion and pull request #254 : we are running Mantis 2.10.0 on Windows (PHP 5.6.17 and Source Subversion Integration plugin 2.1.0). The script (SourceSVN.php) used "w" for the third pipe and we too encountered every now and then hangs (until timeout kicks in after a couple of minutes). This behavior is reproducible when the same SVN revision is used. Seems to be related to the size of the commit. Changing from "w" to "a" seems to solved the issue (I tested it with a SVN revision that would hang before. Now it works.) |
I'm having problem with import script hang. Sometimes very small repo can be imported (10 changesets) but often it hangs. Problem started when i updated two years instalation to current (I updated linux, mantis and all plugins in one go, figures) Fixed with #254 |
The change from "w" to "a" on the third array fixed the issue for me too. The script was loading without log a long time. After changing the character the import was possbile and successful for many repositories. |
Ubuntu 17, mantis 2.20, - had to change the 'w' to an 'a' as well, otherwise it just hangs |
While we found a fix for ourself, it seems still not solved in the long run. Any ideas how we can solve this once and for all? |
This is my attempt to address the issue in a platform independent way. The main idea is to get rid of It seemed to work OK from the limited amount of testing that I was able to do, but it was not sufficient to satisfy me that it's completely robust. Unfortunately I'm no longer using Mantis, so addressing this is languishing down on my task list. |
Looks promising. Hopefully this (or anyother approach) will be included soon so that any future plugin update won't stop this function from working correctly. |
@bright-tools sorry to hear you stopped using Mantis, and thank you for posting the link with your feature branch. Hopefully one or more of the growing number of people who land here, will take things over and get this code to a mergeable state. |
Using the latest 2.1 version (because I'm on IIS 10) when I try and Import Latest Data, the page returned just says 'Import Results'
There are no import results or errors. Note that this is true whether I specify the URL and WebSVN URL correctly or not.
I.e. Even if I set the URL to https:\madeup, it still just says 'Import Results' with no error.
The text was updated successfully, but these errors were encountered: