composer-stager icon indicating copy to clipboard operation
composer-stager copied to clipboard

Speed up begin stage

Open cherbst opened this issue 10 months ago • 8 comments

Right now it is not possible to execute the begin staging using an existing staging directory. But using rsync it would be totally possible to update an existing staging directory and thus speedup the begin stage. Why does this restriction exist and can we drop it?

cherbst avatar Jan 20 '25 10:01 cherbst

It's just to prevent you from accidentally overwriting an existing application, causing corruption or data loss. I think it's a firm requirement. However, since you apparently know what you're doing, you could easily get around the limitation by using the stage operation instead, since that's really what you're doing from a technical perspective. Would that work for you?

TravisCarden avatar Jan 21 '25 21:01 TravisCarden

I am not sure I understand that correctly. Do you mean I should skip the begin operation the second time and just use stage? But then the changes from the active directory wouldn't be synchronized back to the staging directory, right? But that would be necessary.

cherbst avatar Jan 22 '25 10:01 cherbst

Oops; I meant to say commit, not stage. Since you already have a staging directory, you don't need to begin anymore--that's effectively already been done. What you're doing now from a technical perspective is essentially committing the active directory to the staging directory. So you'll just commit in both directions:

sequenceDiagram
    Active Dir->>+Staging Dir: begin (already happened)
    Active Dir->>+Active Dir: live changes apparently?
    Active Dir->>+Staging Dir: commit (instead of begin)
    Staging Dir->>+Staging Dir: stage
    Staging Dir->>+Active Dir: commit

TravisCarden avatar Jan 22 '25 16:01 TravisCarden

Thanks for your detailed explanation TravisCarden! If I understand correctly the first commit switches the active and staging directories, right? So it treats the active directory as the staging directory and vice versa. I think this solution would work very well for me but unfortunately, it seems that the commit command does not respect the exclude options. At least for me it copies over the whole staging directory (the actual active directory). Is this my fault (I am using my fork of the console command: https://github.com/konnektiv/composer-stager-console/tree/include-dirs) or is this by design?

cherbst avatar Jan 30 '25 08:01 cherbst

the first commit switches the active and staging directories, right?

It syncs them, if that's what you mean. In simple terms, the first commit--from staging to active--will update and overwrite the staging directory with the active directory.

it seems that the commit command does not respect the exclude options.

No, it definitely respects exclusions. You can see it working in my automated tests: https://github.com/php-tuf/composer-stager/blob/develop/tests/EndToEnd/EndToEndFunctionalTest.php#L251. The problem seems likely to be in your implementation.

TravisCarden avatar Jan 30 '25 12:01 TravisCarden

Thank you very much, that worked. You were right, the exclusions not being correctly used was a problem with my extensions to the composer-stager-console project. I fixed it here:

https://github.com/php-tuf/composer-stager-console/pull/126/commits/03476e7f9fa765a7c20679b2b1872ed1f8c18cac

cherbst avatar Feb 17 '25 13:02 cherbst

I am sorry, that was to fast. Unfortunately it is a little more complicated. But only because I added the extension --include-dir to the composer-stager-console command. It is used to compute the excluded directories by excluding everything but the directories specified by --include-dir. For this to work it is necessary to know which one is the real active directory. When we are switching the semantics of the options like you suggested, this does no longer work. Of course I could add another option so the user has to specify which is the real active directory. But in my opinion it would be actually better to add an option to drop that restriction that the staging directory must not exist in the begin command, so everything would just work.

cherbst avatar Feb 17 '25 14:02 cherbst

It seems like my initial proposal exposes you to too many unintended consequences from semantic misalignment. What if you override the service instead? It would look something like this:

    PhpTuf\ComposerStager\API\Precondition\Service\ActiveDirExistsInterface:
        class: PhpTuf\ComposerStagerConsole\Precondition\Service\NullPrecondition # <- You'll have to create this yourself for now.

You'll have to create your own NullPrecondition for now. I've created https://github.com/php-tuf/composer-stager/issues/409 to provide one out-of-the-box in the future.

TravisCarden avatar Feb 17 '25 16:02 TravisCarden