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
Suspected cache issue with conveyor #101
Comments
You've done a deep dive into the package to find the error, thanks for that! Unfortunately I cannot easily reproduce the caching issue. If you are able to better pinpoint where the issue lies we might be able to find a workaround. |
Able to help explain how Just for info, your packaged worked fine on Centos 6.4 |
rewriteRules is part of the skeleton. |
I was bothered with the exact same issue and error-message on that block while trying the package (I learned it through laracasts this week). Sadly it didn't work on my vagrant box (laravel/homestead v10) as well on an Ubuntu 20.04 host OS, ending in the same error:
While fiddling a bit inside the package I found something... I evaluated the existence of the files to rename in a while loop. It seems it took about 0.20 seconds over here before the files actually were physically "present" here. Only then they could further properly be renamed by the script and then works fine without any exception error any longer... I added some dumps & timing measurement inbetween (can be dropped) in the test code below, just to visualize the output showing the issue & to get obtain more info. Iterating 10000 times is perhaps a bit of a crude "workaround" but at least, it then works... (after around iterating 750 times up to 1500 on my computer, the files are now being found and can be renamed by then...). I guess it might vary (computer/os dependent), it will need some further elaboration to avoid this depencency... $rewrites = require ($manifest === null) ? [
'src/MyPackage.php' => 'src/:uc:package.php',
'config/mypackage.php' => 'config/:lc:package.php',
'src/Facades/MyPackage.php' => 'src/Facades/:uc:package.php',
'src/MyPackageServiceProvider.php' => 'src/:uc:packageServiceProvider.php',
] : $manifest;
foreach ($rewrites as $file => $name) {
$filename = str_replace($bindings[0], $bindings[1], $name);
/* Checking if file exists and evaluate when it's there... */
$time_start = microtime(true);
$x = 0;
while ($x < 10000) {
$dumptxt = "Searching file..." . $file . (file_exists($this->packagePath().'/'.$file) ? " FOUND " : " NOT FOUND ");
dump($dumptxt);
if(file_exists($this->packagePath().'/'.$file))
{
$dumptxt = "File " . $file . " found after " . $x . " iterations. Time required: " . (microtime(true) - $time_start) . " seconds";
dump($dumptxt);
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
break;
}
$x++;
}
/* End evaluation */
// rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
} Output result (of dumps) gave me the following:
|
@smoetje thanks for providing the info. Let me have a look. Meanwhile possible to edit your earlier post to highlight the PHP syntax? You can do that by adding PHP behind the opening triple ticks i.e. ```PHP, makes it much easier to read haha. I'm pasting in this post just to make it easier for me. $rewrites = require ($manifest === null) ? [
'src/MyPackage.php' => 'src/:uc:package.php',
'config/mypackage.php' => 'config/:lc:package.php',
'src/Facades/MyPackage.php' => 'src/Facades/:uc:package.php',
'src/MyPackageServiceProvider.php' => 'src/:uc:packageServiceProvider.php',
] : $manifest;
foreach ($rewrites as $file => $name) {
$filename = str_replace($bindings[0], $bindings[1], $name);
/* Checking if file exists and evaluate when it's there... */
$time_start = microtime(true);
$x = 0;
while ($x < 10000) {
$dumptxt = "Searching file..." . $file . (file_exists($this->packagePath().'/'.$file) ? " FOUND " : " NOT FOUND ");
dump($dumptxt);
if(file_exists($this->packagePath().'/'.$file))
{
$dumptxt = "File " . $file . " found after " . $x . " iterations. Time required: " . (microtime(true) - $time_start) . " seconds";
dump($dumptxt);
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
break;
}
$x++;
}
/* End evaluation */
// rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
} |
Thanks for the highlight syntax hint. I now tried the same my old laptop with Windows 10: issue occurred as well. As you proposed I changed to a 30 second timeout. Then I simulated with the same test-dumps to grab some timings, got this to work properly as well:
Removing all the overhead dump clutter, I now got this: /**
* Rename generic files to package-specific ones.
*
* @param array|null $manifest
* @return void
**/
public function renameFiles($manifest = null)
{
$bindings = [
[':uc:vendor', ':uc:package', ':lc:vendor', ':lc:package'],
[$this->vendorStudly(), $this->packageStudly(), strtolower($this->vendor()), strtolower($this->package())],
];
$rewrites = require ($manifest === null) ? [
'src/MyPackage.php' => 'src/:uc:package.php',
'config/mypackage.php' => 'config/:lc:package.php',
'src/Facades/MyPackage.php' => 'src/Facades/:uc:package.php',
'src/MyPackageServiceProvider.php' => 'src/:uc:packageServiceProvider.php',
] : $manifest;
foreach ($rewrites as $file => $name) {
$filename = str_replace($bindings[0], $bindings[1], $name);
/* Adding 30 sec timeout, wait for file to exist before renaming */
$start_time = time();
while(true) {
if(file_exists($this->packagePath().'/'.$file) || ((time() - $start_time) > 30)) break;
}
/* End */
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
}
} |
I agree, better let it sleep to free up some time for other resources instead of pointless loops. while(true) {
if(file_exists($this->packagePath().'/'.$file) || ((time() - $start_time) > 30))
{
sleep(1);
break;
}
} |
Oops, sorry, I write in a number of different languages, 1 second was what I meant. :) |
btw, sleep has to be outside the braces while(true) {
if(file_exists($this->packagePath().'/'.$file) || ((time() - $start_time) > 30)) break;
sleep(1);
} |
Why exactly? Did you perhaps try it earlier with a FOR loop? Originally I tried with FOR-loop as I prefer this instead of WHILE(true). However inside the FOR-loop (commented below), it triggers back the same Exception issue after immediately exiting even though the file_exists($this->packagePath()) evaluates as true. This is bizarre for me, and worse: even the extra sleep(1) before break (commented out), it doesn't work here... ...when I decrease the sleep timer to usleep(20000) (20 milliSeconds) just after this FOR loop however, I am getting the same Exception error again which I don't get with the while-version earlier mentioned above... (even with or without sleeping 1 sec) foreach ($rewrites as $file => $name) {
$filename = str_replace($bindings[0], $bindings[1], $name);
/* Setting 30 sec timeout: wait for file to exist before renaming */
for($retries = 0; $retries < 30; $retries++) {
if(file_exists($this->packagePath())) {
// sleep(1); // -> works, usleep(20000) -> doesn't work
break 1;
}
}
sleep(1); // -> works, usleep(20000) -> doesn't work
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
} So for now the only "reliable" way it works, is the earlier mentioned WHILE(true) option. Even without sleep(1) it just... keeps working. |
Ok, let's talk pseudo code first. Here's what we want:
However, the
The
Make sense? I think the foreach ($rewrites as $file => $name) {
$filename = str_replace($bindings[0], $bindings[1], $name);
/* Setting 30 sec timeout: wait for file to exist before renaming */
for($retries = 0; $retries < 30; $retries++) {
if(file_exists($this->packagePath())) {
break; // file exists, let's get out
}
else
{
// take a break and before checking if file exists (we will do this 30 times)
sleep(1);
}
}
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
} |
I get your point, I prefer the FOR loop myself. But as told earlier: the FOR loop isn't working.
I put some dumps inside to observe what happens in the artisan command (see above) /* Setting 30 sec timeout: wait for file to exist before renaming */
for($retries = 0; $retries < 30; $retries++) {
if(file_exists($this->packagePath())) {
dump("File is existing, renaming now!");
break; // file exists, let's get out
}
else
{
// take a break and before checking if file exists (we will do this 30 times)
sleep(1);
}
}
dump("Renaming...");
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename); The WHILE loop doesn't give that issue. If you also give a 1 sec break in the WHILE, it will just evaluate 30 times as well thanks to time() - $start_time) > 30. In any case: when I try the artisan commands this consistently works perfectly, even if it's a bit unconventional. |
Hmm, this is extremely strange. But then again, it was a weird issue to begin with. |
In any case, the reason why it is so extremely strange is because it seems that in some situations when file_exists($this->packagePath()) evaluates to TRUE it still needs an amount of time (milliseconds) before files physically available on the drive. If evaluated (by rename) inside this time-window by PHP, it triggers the Exception. That's my point I am trying to explain here. Artificially adding a delay after the FOR does help, but the time-window varies so sometimes it works, sometimes it doesn't. Yeah, it's strange for me as well... |
Hmm, does this work? for($retries = 0; $retries < 30; $retries++) {
if(file_exists($this->packagePath())) {
// file exists, but take a break and before getting out and hope that file physically exists
sleep(1);
break;
}
else
{
// take a break and before checking if file exists (we will do this 30 times)
sleep(1);
}
} |
This code does work. However, the time sleep(1) remains to be the "weak spot" inside the given IF statement. Will 1 second suffice for all possible cases? I hope, but still consider this way of coding as being "riskier" for exceptions... if(file_exists($this->packagePath())) {
// file exists, but take a break and before getting out and hope that file physically exists
sleep(1); // --> Weak point: time varies when files are available for renaming, this "makes it or breaks it"...
break;
} Output (several attempts, with different sleep times)
|
I gave it some thought. Since only for($retries = 0; $retries < 30; $retries++) {
try
{
if(file_exists($this->packagePath())) {
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
break; // rename did not throw exception, get out of the for loop
}
}
catch (\Exception $e)
{
// take a break before checking if file exists (we will do this 30 times)
sleep(1);
}
} |
The exception is a good idea. I tried your code... and sadly, there's the same error again:
I added some extra inline dumps to observe the in-code behavior: for($retries = 0; $retries < 30; $retries++) {
try
{
if(file_exists($this->packagePath())) {
dump("File exists?");
dump(file_exists($this->packagePath()));
dump("Attempt nr evaluating TRUE : " . $retries);
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
break; // rename did not throw exception, get out of the for loop
}
}
catch (\Exception $e)
{
dump("Exception catched on attempt " . $retries);
// take a break before checking if file exists (we will do this 30 times)
sleep(1);
}
} Attempt 0: Attempt 1: |
I can't see the try {
rename("randomfilename1", "randomfilename2");
} catch (\Exception $e) {
$this->line("caught");
} |
@smoetje based on your error logs, I'm quite sure it is not the code I proposed. There is a |
@smoetje In the interest of moving forward, can replace your public function renameFiles($manifest = null)
{
$bindings = [
[':uc:vendor', ':uc:package', ':lc:vendor', ':lc:package'],
[$this->vendorStudly(), $this->packageStudly(), strtolower($this->vendor()), strtolower($this->package())],
];
for ($timeout = 30; $timeout > 0; $timeout--) {
try {
$rewrites = require ($manifest === null) ? [
'src/MyPackage.php' => 'src/:uc:package.php',
'config/mypackage.php' => 'config/:lc:package.php',
'src/Facades/MyPackage.php' => 'src/Facades/:uc:package.php',
'src/MyPackageServiceProvider.php' => 'src/:uc:packageServiceProvider.php',
] : $manifest;
break;
} catch (\Exception $e) {
// take a break before trying again, we will do this until timeout
// this is to overcome a weird CentOS 7 caching issue
// see https://github.com/Jeroen-G/laravel-packager/issues/101
sleep(1);
}
}
foreach ($rewrites as $file => $name) {
$filename = str_replace($bindings[0], $bindings[1], $name);
for ($timeout = 30; $timeout > 0; $timeout--) {
try {
rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
break;
} catch (\Exception $e) {
// dealing with the same caching issue as above
sleep(1);
}
}
}
} |
Short update on the last update. |
The next release will add a timeout config option. Is that helping here too? |
Not sure what the timeout config option does, but the functions that threw the intermittent exceptions here are |
That's what i posted in the mentioned pull request.
|
I have exactly no idea what's going on here. But i think the timeout is not a fix for this problem. I think we have a race condition. Files should be renamed/replaced but they are not there in that moment. Maybe the unzipping has not been finished or something else. |
@Schnoop yes, I'm on VirtualBox 6.0.14r133895, but I have since moved onto CentOS 8 (from CentOS 7) due to other issues. But I haven't created any packages recently, so I'm not entirely sure. If you examine the PHP code, |
The complete function has been rewritten. There is a
|
@Schnoop I'm not where what to be looking for in |
Nope. The rename will only be executed if a replace has been done. In fact the RecursiveDirectoryIterator find the following files:
As you can see the src folder is found, but all files in that folder are missing. Now the foreach kicks in, and iterates over that found set of files. As those files don't have to rename the rename method will never been reached as the 3 lines before will prevent that. |
If if pause the renameFiles Method in the first line with a
All files in subdirs are found and accessible - thus renamable. All fine. So i think this is a filesystem problem. |
|
|
I narrowed down the solution to adding a I could continue and find a better way to test for the problem, but based on what laravel-packager/src/ArchiveExtractors/Zip.php Lines 14 to 17 in ab358d9
|
In tracing the issue, I also modified the function below to produce the 2 outputs public function renameFiles($manifest = null)
{
$bindings = [
['MyVendor', 'MyPackage', 'myvendor', 'mypackage'],
[$this->vendorStudly(), $this->packageStudly(), strtolower($this->vendor()), strtolower($this->package())],
];
$files = new RecursiveDirectoryIterator($this->packagePath());
foreach (new RecursiveIteratorIterator($files) as $file) {
echo $file->getPathName(). "\r\n";
if (! $file->isFile()) {
echo "- Not file\r\n";
continue;
}
$replaced = str_replace($bindings[0], $bindings[1], $file->getFilename());
if ($replaced === $file->getFilename()) {
echo "- Same\r\n";
continue;
}
$src = $file->getPath().'/'.$file->getFilename();
$dst = $file->getPath().'/'.$replaced;
echo "- Renaming $src => $dst\r\n";
rename($src, $dst);
// rename($file->getPath().'/'.$file->getFilename(), $file->getPath().'/'.$replaced);
}
} Below is output with
Below is output without
|
Hi,
I keep getting an
No such file or directory
ErrorException
on this linelaravel-packager/src/FileHandler.php
Line 180 in 5be28b9
However, if I set a xdebug breakpoint at the
require
here and continue running, noErrorException
is thrownlaravel-packager/src/FileHandler.php
Line 175 in 5be28b9
This leads me to suspect that there is a caching issue when running on CentOS Linux release 7.7.1908 (Core), is this something you can fix?
I believe the issue is in the
downloadSkeleton()
laravel-packager/src/Commands/NewPackage.php
Line 91 in 46e4cda
Thank you.
The text was updated successfully, but these errors were encountered: