Skip to content
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

Open
bilogic opened this issue Mar 2, 2020 · 39 comments
Open

Suspected cache issue with conveyor #101

bilogic opened this issue Mar 2, 2020 · 39 comments
Labels
needs work postponed Needs more thinking and elaboration.

Comments

@bilogic
Copy link
Contributor

bilogic commented Mar 2, 2020

Hi,

I keep getting an No such file or directory ErrorException on this line

] : $manifest;

However, if I set a xdebug breakpoint at the require here and continue running, no ErrorException is thrown

$rewrites = require ($manifest === null) ? [

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()

$this->conveyor->downloadSkeleton();

Thank you.

@Jeroen-G
Copy link
Owner

Jeroen-G commented Mar 2, 2020

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?

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.

@bilogic
Copy link
Contributor Author

bilogic commented Mar 2, 2020

Able to help explain how downloadSkeleton() works?
Specifically, where does it write the file rewriteRules.php?
Perhaps there was a fclose() somewhere that was not called?

Just for info, your packaged worked fine on Centos 6.4

@Jeroen-G
Copy link
Owner

Jeroen-G commented Mar 2, 2020

rewriteRules is part of the skeleton.
DownloadSkeleton() downloads that skeleton as a zip, extracts it to a temporary folder and then moves the files in that folder to the package folder.

@smoetje
Copy link

smoetje commented Sep 20, 2020

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:

vagrant@homestead:~/code/pkg3$ artisan packager:new zizi/zaza37
 0/6 [>---------------------------]   0% Creating package zizi\zaza37...
Creating packages directory...
Creating vendor...
Downloading skeleton...

**ErrorException** 

rename(/home/vagrant/code/pkg3/packages/zizi/zaza37/src/MyPackage.php,/home/vagrant/code/pkg3/packages/zizi/zaza37/src/Zaza37.php): **No such file or directory**

at vendor/jeroen-g/laravel-packager/src/FileHandler.php:199
➜ 199▕             rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);

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:

"Searching file...src/MyPackage.php NOT FOUND "
"Searching file...src/MyPackage.php NOT FOUND "
"Searching file...src/MyPackage.php NOT FOUND "
"Searching file...src/MyPackage.php NOT FOUND "
"Searching file...src/MyPackage.php NOT FOUND "
"File src/MyPackage.php found after 1419 iterations. Time required: 0.19641780853271 seconds"
"Searching file...config/mypackage.php FOUND "
"File config/mypackage.php found after 0 iterations. Time required: 0.00022220611572266 seconds"
"Searching file...src/Facades/MyPackage.php FOUND "
"File src/Facades/MyPackage.php found after 0 iterations. Time required: 0.00017499923706055 seconds"
"Searching file...src/MyPackageServiceProvider.php FOUND "
"File src/MyPackageServiceProvider.php found after 0 iterations. Time required: 0.00014495849609375 seconds"
 4/6 [==================>---------]  66% Replacing skeleton placeholders...
Installing package...
 6/6 [============================] 100% Package created successfully!

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

@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);
        }

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

@Jeroen-G @smoetje I consider the root of this issue hard to replicate and not exactly our "job".
May I propose we replace the 10000 cycle count with a 30 second timeout?

@smoetje
Copy link

smoetje commented Sep 21, 2020

@Jeroen-G @smoetje I consider the root of this issue hard to replicate and not exactly our "job".
May I propose we replace the 10000 cycle count with a 30 second timeout?

Thanks for the highlight syntax hint.
I meanwhile changed it :)

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:

"Searching file...src/MyPackage.php NOT FOUND "
"Searching file...src/MyPackage.php NOT FOUND "
"Searching file...src/MyPackage.php NOT FOUND "
"File src/MyPackage.php found after 718 iterations. Time required: 0.19298696517944 seconds"
"Searching file...config/mypackage.php FOUND "
"File config/mypackage.php found after 1 iterations. Time required: 0.00087594985961914 seconds"
"Searching file...src/Facades/MyPackage.php FOUND "
"File src/Facades/MyPackage.php found after 1 iterations. Time required: 0.00060296058654785 seconds"
"Searching file...src/MyPackageServiceProvider.php FOUND "
"File src/MyPackageServiceProvider.php found after 1 iterations. Time required: 0.00055408477783203 seconds"
 4/6 [==================>---------]  66% Replacing skeleton placeholders...
Installing package...
 6/6 [============================] 100% Package created successfully!

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);
        }
    }

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

@smoetje cool!

  1. Maybe add a sleep(1000); after the break; inside the while loop to slow things down a bit?
  2. I submitted a PR to not too long ago. Maybe see if @Jeroen-G is ok with this.

@smoetje
Copy link

smoetje commented Sep 21, 2020

  1. Maybe add a sleep(1000); after the break; inside the while loop to slow things down a bit?

I agree, better let it sleep to free up some time for other resources instead of pointless loops.
sleep(1000) might however be a tad too long I think.
Better limit to 1 (it's in second according to the docs). I checked and it works fine a well.

            while(true) {
                if(file_exists($this->packagePath().'/'.$file) || ((time() - $start_time) > 30))
                {
                    sleep(1);
                    break;
                }
            }

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

Oops, sorry, I write in a number of different languages, 1 second was what I meant. :)

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

btw, sleep has to be outside the braces

while(true) {
    if(file_exists($this->packagePath().'/'.$file) || ((time() - $start_time) > 30)) break;
    sleep(1);
}

@smoetje
Copy link

smoetje commented Sep 21, 2020

btw, sleep has to be outside the braces

Why exactly? Did you perhaps try it earlier with a FOR loop?
My original purpose for looping hundreds (or thousands) of times checking for file existence was to check for file presence and get timing (because I suspected delayed file write). But your proposal for one check per second in this case makes sense to free up computing resources. So it's checked up to 30 times now and breaks when the condition is met, you'll also get 1 additional second in the last while iteration before exiting the 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 putting the sleep(1) after the for FOR-loop, then it works... BUT

...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)
So I cannot rely on that one:

        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.

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

Ok, let's talk pseudo code first.

Here's what we want:

  1. loop
  2. if we have reached 30 seconds OR file exists, get out of loop
  3. since we are still in loop, take a break for 1 second to avoid spinning
  4. go back to start of loop

However, the while(true) loop was doing:

  1. loop
  2. if we have reached 30 seconds OR file exists, then sleep 1 second and get out of loop
  3. go back to start of loop
  • The issue here is, it only sleeps 1 second right before getting out of the loop, rest of the time, it is spinning which causes high CPU load.

The for loop now does:

  1. loop for 30 times
  2. if file exists, then get out of loop
  3. go back to start of loop
  • The issue here is, it never sleeps, it keeps spinning which causes high CPU load.

Make sense?

I think the for loop is cleaner, but needs some changes below:

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);
}

@smoetje
Copy link

smoetje commented Sep 21, 2020

I get your point, I prefer the FOR loop myself.

But as told earlier: the FOR loop isn't working.
Just tried your changes, can reproduce the same issue...

vagrant@homestead:~/code/pkg$ artisan packager:new smoetje testpackage21
 0/6 [>---------------------------]   0% Creating package smoetje\testpackage21...
Creating packages directory...
Creating vendor...
Downloading skeleton...
"File is existing, renaming now!"
"Renaming..."

   ErrorException

  rename(/home/vagrant/code/pkg/packages/smoetje/testpackage21/src/MyPackage.php,/home/vagrant/code/pkg/packages/smoetje/testpackage21/src/Testpackage21.php): No such file or directory

  at vendor/jeroen-g/laravel-packager/src/FileHandler.php:210
    206▕                 }
    207▕             }
    208▕
    209▕             dump("Renaming...");
  ➜ 210▕             rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
    211▕         }
    212▕     }
    213▕
    214▕     /**

      +16 vendor frames
  17  artisan:37
      Illuminate\Foundation\Console\Kernel::handle()

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.
So it's not endlessly eating up processing time.

In any case: when I try the artisan commands this consistently works perfectly, even if it's a bit unconventional.
That's what matters? ;)

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

Hmm, this is extremely strange. But then again, it was a weird issue to begin with.
I think you understand where to put the sleep(1). Hope you can fix the problem for us! :)

@smoetje
Copy link

smoetje commented Sep 21, 2020

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.
When using the (unconventional) WHILE loop, it works perfect and consistent (and even no delay is required...) and fixes the issue.

Yeah, it's strange for me as well...

@bilogic
Copy link
Contributor Author

bilogic commented Sep 21, 2020

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);
                }
            }

@smoetje
Copy link

smoetje commented Sep 21, 2020

This code does work. However, the time sleep(1) remains to be the "weak spot" inside the given IF statement.
Right now no one can predict how long it takes for the files to be present for renaming. My measurements done earlier (see above) varied between 0,19 seconds (approx 750 cycles) and 0,40 seconds (approx 1500 cycles)...

Will 1 second suffice for all possible cases? I hope, but still consider this way of coding as being "riskier" for exceptions...
Increasing this time more will undoubtfully reduce the risk more, but in exchange sacrificing some time/performance.

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)

vagrant@homestead:~/code/pkg$ artisan packager:new smoetje test_original
 0/6 [>---------------------------]   0% Creating package smoetje\test_original...
Creating packages directory...
Creating vendor...
Downloading skeleton...

   ErrorException 
  rename(/home/vagrant/code/pkg/packages/smoetje/test_original/src/MyPackage.php,/home/vagrant/code/pkg/packages/smoetje/test_original/src/TestOriginal.php): No such file or directory

  at vendor/jeroen-g/laravel-packager/src/FileHandler.php:187
    183▕             $filename = str_replace($bindings[0], $bindings[1], $name);
    184▕ 
    185▕             
    186▕ 
  ➜ 187▕             rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
    188▕         }
    189▕     }
    190▕ 
    191▕     /**

      +16 vendor frames 
  17  artisan:37
      Illuminate\Foundation\Console\Kernel::handle()
vagrant@homestead:~/code/pkg$ artisan packager:new smoetje test_bilogic_mod1
 0/6 [>---------------------------]   0% Creating package smoetje\test_bilogic_mod1...
Creating packages directory...
Creating vendor...
Downloading skeleton...
 4/6 [==================>---------]  66% Replacing skeleton placeholders...
Installing package...
 6/6 [============================] 100% Package created successfully!


vagrant@homestead:~/code/pkg$ artisan packager:new smoetje test_bilogic_mod2
 0/6 [>---------------------------]   0% Creating package smoetje\test_bilogic_mod2...
Creating packages directory...
Creating vendor...
Downloading skeleton...

   ErrorException 
  rename(/home/vagrant/code/pkg/packages/smoetje/test_bilogic_mod2/src/MyPackage.php,/home/vagrant/code/pkg/packages/smoetje/test_bilogic_mod2/src/TestBilogicMod2.php): No such file or directory

  at vendor/jeroen-g/laravel-packager/src/FileHandler.php:198
    194▕                     sleep(1);
    195▕                 }
    196▕             }
    197▕ 
  ➜ 198▕             rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
    199▕         }
    200▕     }
    201▕ 
    202▕     /**

      +16 vendor frames 
  17  artisan:37
      Illuminate\Foundation\Console\Kernel::handle()
vagrant@homestead:~/code/pkg$ artisan packager:new smoetje test_bilogic_mod3
 0/6 [>---------------------------]   0% Creating package smoetje\test_bilogic_mod3...
Creating packages directory...
Creating vendor...
Downloading skeleton...
 4/6 [==================>---------]  66% Replacing skeleton placeholders...
Installing package...
 6/6 [============================] 100% Package created successfully!

@bilogic
Copy link
Contributor Author

bilogic commented Sep 22, 2020

I gave it some thought.

Since only rename() is the most reliable, let's ignore the exception for up to 30 seconds.
We can certainly filter to only the "No such file or directory" exception and throw any other exception, but that code has to be tested and I don't have a setup at the moment.

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);
    }
}

@smoetje
Copy link

smoetje commented Sep 22, 2020

The exception is a good idea. I tried your code... and sadly, there's the same error again:

vagrant@homestead:~/code/pkg$ artisan packager:new smoetje bilogic_test8
 0/6 [>---------------------------]   0% Creating package smoetje\bilogic_test8...
Creating packages directory...
Creating vendor...
Downloading skeleton...
"File exists?"
true
"Attempt nr evaluating TRUE : 0"
"Exception catched on attempt 0"
"File exists?"
true
"Attempt nr evaluating TRUE : 1"

   ErrorException 

  rename(/home/vagrant/code/pkg/packages/smoetje/bilogic_test8/src/MyPackage.php,/home/vagrant/code/pkg/packages/smoetje/bilogic_test8/src/BilogicTest8.php): No such file or directory

  at vendor/jeroen-g/laravel-packager/src/FileHandler.php:204
    200▕                     sleep(1);
    201▕                 }
    202▕             }
    203▕ 
  ➜ 204▕             rename($this->packagePath().'/'.$file, $this->packagePath().'/'.$filename);
    205▕         }
    206▕     }
    207▕ 
    208▕     /**

      +16 vendor frames 
  17  artisan:37
      Illuminate\Foundation\Console\Kernel::handle()

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:
Evaluation existence of file = TRUE
Trying to rename -> Exception -> Has been catched ordering to sleep 1 second

Attempt 1:
Evaluation existence of file = TRUE
Trying to rename -> Exception NOT occurring but when trying to rename file -> Error + Exception is not being catched...

@bilogic
Copy link
Contributor Author

bilogic commented Sep 23, 2020

I can't see the catch statement on line 207 of your output, are you sure it is there?
Just to be sure and safe, I tested and was able catch on both L5.8.38 and L8.6.0

    try {
        rename("randomfilename1", "randomfilename2");
    } catch (\Exception $e) {
        $this->line("caught");
    }

@bilogic
Copy link
Contributor Author

bilogic commented Sep 23, 2020

@smoetje based on your error logs, I'm quite sure it is not the code I proposed. There is a sleep(1) on line 200, there should not be any sleep() above rename().

@bilogic
Copy link
Contributor Author

bilogic commented Sep 23, 2020

@smoetje In the interest of moving forward, can replace your renameFiles() with my code below?
Btw, we actually faced different issues, mine was $manifest, while yours was rename()

    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);
                }
            }
        }
    }

@smoetje
Copy link

smoetje commented Sep 30, 2020

Short update on the last update.
I have tried on several VM setups with this version (I work on multiple machines). I took the liberty to make some tests and it also looks like the exception is being properly properly caught when the rename fails. I also noticed that when using dumps in between (to debug/visualize some data & metrics) it may have been a possible cause of the unexpected behavior.
So in short: this one seems to be working fine here...

@bilogic
Copy link
Contributor Author

bilogic commented Oct 1, 2020

Thanks @smoetje!

@Jeroen-G I filed a PR earlier. let me know if there are any issues. Thank you.

@Jeroen-G
Copy link
Owner

Jeroen-G commented Oct 9, 2020

The next release will add a timeout config option. Is that helping here too?

@Jeroen-G Jeroen-G added needs work postponed Needs more thinking and elaboration. labels Oct 9, 2020
@bilogic
Copy link
Contributor Author

bilogic commented Oct 10, 2020

@Jeroen-G

Not sure what the timeout config option does, but the functions that threw the intermittent exceptions here are require and rename. I can't think of any timeout mechanism that relates to them.

@Schnoop
Copy link

Schnoop commented Oct 27, 2020

That's what i posted in the mentioned pull request.

I've the same issue on Laravel Homestead. It looks like the files not found while trying to rename them. I'm ending with a useless package that looks like the downloaded skeleton.
If i slow down the command, e.g put a sleep(5); on the first line of the renameFiles function everything works well. Laravel Homestead uses Ubuntu as OS (18.04.5 LTS (Bionic Beaver)). Looks like this isn't a CentOS only problem.

@Schnoop
Copy link

Schnoop commented Oct 27, 2020

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.
@bilogic on what platform do you have this problem? Maybe it is virtualbox/vmware related? I use Laravel Homestead which, in my case, is a virtualbox image. Virtual file systems are way slower than a native one.

@bilogic
Copy link
Contributor Author

bilogic commented Oct 27, 2020

@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, exists() passes but rename() fails. As those codes fall outside debuggable PHP, I did not pursue further for the lack of skill.

@Schnoop
Copy link

Schnoop commented Oct 27, 2020

The complete function has been rewritten. There is a RecursiveDirectoryIterator in place by now and this one didn't return all files. So in fact those files will never tried to rename. Have a look:

    /**
     * Rename generic files to package-specific ones.
     *
     * @param array|null $manifest
     * @return void
     **/
    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) {
            if (! $file->isFile()) {
                continue;
            }
            $replaced = str_replace($bindings[0], $bindings[1], $file->getFilename());
            if ($replaced === $file->getFilename()) {
                continue;
            }
            rename($file->getPath().'/'.$file->getFilename(), $file->getPath().'/'.$replaced);
        }
    }

@bilogic
Copy link
Contributor Author

bilogic commented Oct 27, 2020

@Schnoop I'm not where what to be looking for in renameFiles(), but the rename() is still there, possibly still able to cause this issue.

@Schnoop
Copy link

Schnoop commented Oct 27, 2020

Nope. The rename will only be executed if a replace has been done. In fact the RecursiveDirectoryIterator find the following files:

[2020-10-27 12:02:00] production.ERROR: .  
[2020-10-27 12:02:00] production.ERROR: ..  
[2020-10-27 12:02:00] production.ERROR: .styleci.yml  
[2020-10-27 12:02:00] production.ERROR: rewriteRules.php  
[2020-10-27 12:02:00] production.ERROR: contributing.md  
[2020-10-27 12:02:00] production.ERROR: readme.md  
[2020-10-27 12:02:00] production.ERROR: license.md  
[2020-10-27 12:02:00] production.ERROR: config  
[2020-10-27 12:02:00] production.ERROR: phpunit.xml  
[2020-10-27 12:02:00] production.ERROR: rules.php  
[2020-10-27 12:02:00] production.ERROR: composer.json  
[2020-10-27 12:02:00] production.ERROR: src  
[2020-10-27 12:02:00] production.ERROR: changelog.md  

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.

@Schnoop
Copy link

Schnoop commented Oct 27, 2020

If if pause the renameFiles Method in the first line with a sleep(5); the RecursiveFilesIterator returns this set of files:

[2020-10-27 13:12:29] production.ERROR: .  
[2020-10-27 13:12:29] production.ERROR: ..  
[2020-10-27 13:12:29] production.ERROR: .styleci.yml  
[2020-10-27 13:12:29] production.ERROR: rewriteRules.php  
[2020-10-27 13:12:29] production.ERROR: contributing.md  
[2020-10-27 13:12:29] production.ERROR: readme.md  
[2020-10-27 13:12:29] production.ERROR: license.md  
[2020-10-27 13:12:29] production.ERROR: .  
[2020-10-27 13:12:29] production.ERROR: ..  
[2020-10-27 13:12:29] production.ERROR: mypackage.php  
[2020-10-27 13:12:29] production.ERROR: phpunit.xml  
[2020-10-27 13:12:29] production.ERROR: rules.php  
[2020-10-27 13:12:29] production.ERROR: composer.json  
[2020-10-27 13:12:29] production.ERROR: .  
[2020-10-27 13:12:29] production.ERROR: ..  
[2020-10-27 13:12:29] production.ERROR: MyPackage.php  
[2020-10-27 13:12:29] production.ERROR: .  
[2020-10-27 13:12:29] production.ERROR: ..  
[2020-10-27 13:12:29] production.ERROR: MyPackage.php  
[2020-10-27 13:12:29] production.ERROR: MyPackageServiceProvider.php  
[2020-10-27 13:12:29] production.ERROR: changelog.md  

All files in subdirs are found and accessible - thus renamable. All fine. So i think this is a filesystem problem.

@bilogic
Copy link
Contributor Author

bilogic commented Oct 27, 2020

@Schnoop

  1. I think we agree the issue is outside PHP. Do you know who or how we can identify it?
  2. If sleep(5) solves your problem, what is the difference between it and a 30 second retry & timeout?

@Schnoop
Copy link

Schnoop commented Oct 27, 2020

  1. Nope. I'll try to speed up my virtualbox via NFS, which mentioned here: https://laravel.com/docs/8.x/homestead#configuring-homestead
  2. No difference at all. My sleep was just a debug purpose. I don't know if this is a good approach to fix the problem. Feels a bit hacky.

@bilogic
Copy link
Contributor Author

bilogic commented Oct 27, 2020

@Schnoop yea, so in my view, #2 is the best we can do given #1.

@bilogic
Copy link
Contributor Author

bilogic commented Apr 27, 2021

@Jeroen-G

I narrowed down the solution to adding a sleep(1) after ->extractTo(...). If the sleep(1) was placed before it, the problem persists. The issue is probably due to the files and folders being renamed a number of times leading to the VirtualBox FS unable to keep up or something.

I could continue and find a better way to test for the problem, but based on what composer did to fix their issue, I think sleep(1) is the appropriate balance. What do you think? (We could make it conditional)

$archive = new ZipArchive;
$archive->open($pathToArchive);
$archive->extractTo($pathToDirectory);
$archive->close();

@bilogic
Copy link
Contributor Author

bilogic commented Apr 27, 2021

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 sleep(1)

 0/6 [>---------------------------]   0% Creating package bilogic\deepdive...
Creating packages directory...
Creating vendor...
Downloading skeleton...
/media/sf_new/package-repo/packages/bilogic/deepdive/.
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/..
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/.styleci.yml
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/changelog.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/composer.json
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/config/.
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/config/..
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/config/mypackage.php
- Renaming /media/sf_new/package-repo/packages/bilogic/deepdive/config/mypackage.php => /media/sf_new/package-repo/packages/bilogic/deepdive/config/deepdive.php
/media/sf_new/package-repo/packages/bilogic/deepdive/contributing.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/license.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/phpunit.xml
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/readme.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/rewriteRules.php
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/rules.php
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/src/.
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/src/..
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/src/Facades/.
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/src/Facades/..
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/src/Facades/MyPackage.php
- Renaming /media/sf_new/package-repo/packages/bilogic/deepdive/src/Facades/MyPackage.php => /media/sf_new/package-repo/packages/bilogic/deepdive/src/Facades/Deepdive.php
/media/sf_new/package-repo/packages/bilogic/deepdive/src/MyPackage.php
- Renaming /media/sf_new/package-repo/packages/bilogic/deepdive/src/MyPackage.php => /media/sf_new/package-repo/packages/bilogic/deepdive/src/Deepdive.php
/media/sf_new/package-repo/packages/bilogic/deepdive/src/MyPackageServiceProvider.php
 4/6 [==================>---------]  66% Replacing skeleton placeholders...
Installing package...
 6/6 [============================] 100% Package created successfully!

Below is output without sleep(1), notice how it is missing some entries compared to above

 0/6 [>---------------------------]   0% Creating package bilogic\deepdive...
Creating packages directory...
Creating vendor...
Downloading skeleton...
/media/sf_new/package-repo/packages/bilogic/deepdive/.
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/..
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/.styleci.yml
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/changelog.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/composer.json
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/config
- Not file
/media/sf_new/package-repo/packages/bilogic/deepdive/contributing.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/license.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/phpunit.xml
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/readme.md
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/rewriteRules.php
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/rules.php
- Same
/media/sf_new/package-repo/packages/bilogic/deepdive/src
- Not file
 4/6 [==================>---------]  66% Replacing skeleton placeholders...

   ErrorException  : file_get_contents(/media/sf_new/package-repo/packages/bilogic/deepdive/.styleci.yml): failed to open stream: No such file or directory

  at /media/sf_new/package-repo/vendor/jeroen-g/laravel-packager/src/Wrapping.php:74
    70|     public function fillInFile($template, $destination = null)
    71|     {
    72|         $destination = ($destination === null) ? $template : $destination;
    73| 
  > 74|         $filledFile = str_replace($this->placeholders, $this->replacements, file_get_contents($template));
    75|         file_put_contents($destination, $filledFile);
    76| 
    77|         return $this;
    78|     }

  Exception trace:

  1   file_get_contents()
      /media/sf_new/package-repo/vendor/jeroen-g/laravel-packager/src/Wrapping.php:74

  2   JeroenG\Packager\Wrapping::fillInFile()
      /media/sf_new/package-repo/vendor/jeroen-g/laravel-packager/src/Wrapping.php:59

  Please use the argument -v to see more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs work postponed Needs more thinking and elaboration.
Projects
None yet
Development

No branches or pull requests

4 participants