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

fingerprint unexpected behavior when file has changed after the command run #446

Open
TheoD02 opened this issue Apr 29, 2024 · 2 comments
Open

Comments

@TheoD02
Copy link
Contributor

TheoD02 commented Apr 29, 2024

Hi

I am encountering a logical behavior with the current implementation of fingerprint. I would like to discuss the possibility to handle the case I will demonstrate below.

Here is my use case :

if (! fingerprint(callback: static fn () => composer()->install(), fingerprint: fgp()->composer(), force: $forceVendor || $force)) {
    io()->note('Composer dependencies are already installed.');
} else {
    io()->success('Composer dependencies installed');
}

Simply the code will install the composer dependencies (command is run with --quiet for better visiblity)

The output of what is currently done by the script

image

But in that case, the second run of castor install should not run composer installation. Because in reality the file hasn't changed between the first and second installation

That behavior is actually due to current implementation.

castor/src/functions.php

Lines 587 to 597 in 642a6a9

function fingerprint(callable $callback, string $fingerprint, bool $force = false): bool
{
if ($force || !fingerprint_exists($fingerprint)) {
$callback();
fingerprint_save($fingerprint);
return true;
}
return false;
}

We pass the computed $fingerprint directly on fingerprint method.

The checked and saved fingerprint is the same from start to the end of execution. And it seem that composer update the file without changing anything in it, but the fingerprint change.

image

What i have in mind is something like this :

function fingerprint(callable $callback, callable $fingerprint, bool $force = false, bool $delayed = true): bool
{
    $hash = $fingerprint();
    if ($force || !fingerprint_exists($hash)) {
        $callback();
        fingerprint_save($delayed ? $fingerprint() : $hash);

        return true;
    }

    return false;
}
// Current definition of fingerprint
function composer(): string
{
    return hasher()
        ->writeWithFinder(finder()->files()->in(path('app'))->name(['composer.json', 'composer.lock', 'symfony.lock']))
        ->finish()
    ;
}

// With the new implementation purposed (BC but we can support the two definition until 1.0 ? string|callable ?) 
public function composer(): callable
{
    return function () {
        return hasher()
            ->writeWithFinder(finder()->files()->in(path('app'))->name(['composer.json', 'composer.lock', 'symfony.lock']))
            ->finish()
        ;
    };
}

WDYT about this approach, did you have another idea ? 😄

Waiting for feedbacks

@joelwurtz
Copy link
Member

joelwurtz commented May 3, 2024

I don't like having callback, and i think your behavior can be fixed by using the FileHashStrategy::Content strategy instead of the FileHashStrategy::MTimes one

Also the proposed behavior may induce some bugs, i.e.:

  1. I have a fingerprint on a file
  2. I Update the file
  3. I run a command that launch a very long task when this fingerprint has changed
  4. I update the file before the command finish
  5. I rerun the command after the first one has finished -> no update (but it should have been run)

@TheoD02
Copy link
Contributor Author

TheoD02 commented May 3, 2024

I don't like having callback, and i think your behavior can be fixed by using the FileHashStrategy::Content strategy instead of the FileHashStrategy::MTimes one

Also the proposed behavior may induce some bugs, i.e.:

  1. I have a fingerprint on a file
  2. I Update the file
  3. I run a command that launch a very long task when this fingerprint has changed
  4. I update the file before the command finish
  5. I rerun the command after the first one has finished -> no update (but it should have been run)

Hi, thank for the response

Yes, I had thought of using FileHashStrategy::Content.

But this one can also be misleading, let's admit it:

I'm on the main branch with a file containing ABC => this will play the command then save it as a fingerprint.

Let's say I need to change branch to feat/anything here the file has been updated with XYZ so the command will also run.

But when I return to main the command won't run because ABC content is still in the fingerprint cache too.


Yes I see the use case, which is why I had the idea of including a delayed parameter to indicate that this fingerprint run will be saved in a different way.

But I think the case of user modification during a long task should use flock to prevent modification of this file?

IDK which is the best solution, it all depends on the use case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants