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

[Reproducible Builds] Diff in generated/metadata between the builds #23324

Closed
ihor-sviziev opened this issue Jun 19, 2019 · 16 comments
Closed

[Reproducible Builds] Diff in generated/metadata between the builds #23324

ihor-sviziev opened this issue Jun 19, 2019 · 16 comments
Assignees
Labels
Component: Filesystem Fixed in 2.3.x The issue has been fixed in 2.3 release line Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Jun 19, 2019

It's really important to have Reproducible Builds. It will help to check diff between the builds, so you'll be sure that you deploy only needed changes to production.

Preconditions (*)

  1. Magento 2.2.8 & 2.3.x
  2. Configured Build system on any CI (Bitbucket Pipelines in my case)
  3. When build created - create tar.gz or any other archive and upload it somewhere to make it availabe for downloads later (AWS S3 for instance)

Steps to reproduce (*)

  1. On build system make sure that you're executing php bin/magento setup:di:compile
  2. Execute build preparation multiple times for single commit on two different PCs
  3. Download archives for all builds that you prepared
  4. Unpack archives and compare it with diff -rq ./build-1/ ./build-2/

Expected result (*)

  1. You should have exactly the same results in generated/metadata folders

Actual result (*)

  1. We have diff:
Files build-1/magento2/generated/metadata/crontab.php and build-2/magento2/generated/metadata/crontab.php differ
Files build-1/magento2/generated/metadata/frontend.php and build-2/magento2/generated/metadata/frontend.php differ
Files build-1/magento2/generated/metadata/global.php and build-2/magento2/generated/metadata/global.php differ
Files build-1/magento2/generated/metadata/webapi_rest.php and build-2/magento2/generated/metadata/webapi_rest.php differ
Files build-1/magento2/generated/metadata/webapi_soap.php and build-2/magento2/generated/metadata/webapi_soap.php differ

Additional info

This issue reproducing all the time on Bitbucket Pipelines, but locally I can't reproduce it. I think it caused by using different servers on each build on CI.

If we'll try to compare these files after sorting - they actually the same. I sorted them using following script:

<?php
// Note this method returns a boolean and not the array
function recur_ksort(&$array) {
   foreach ($array as &$value) {
      if (is_array($value)) recur_ksort($value);
   }
   return ksort($array);
}
$result = include 'build-1/magento2/generated/metadata/adminhtml.php';
recur_ksort($result);

echo var_export($result);
@m2-assistant
Copy link

m2-assistant bot commented Jun 19, 2019

Hi @ihor-sviziev. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.3-develop instance - upcoming 2.3.x release

For more details, please, review the Magento Contributor Assistant documentation.

@ihor-sviziev do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@orlangur
Copy link
Contributor

@ihor-sviziev,

Actual result (*)
We have diff:

Please explain.

using RecursiveDirectoryIterator object for generating list of all classes and all their arguments and it could return not sorted result.

This is perfectly valid. However, system behavior must not vary depending on order of generation.

https://reproducible-builds.org/ has nothing to do with this issue: it's all about no backdoors in compilers.

@ihor-sviziev
Copy link
Contributor Author

Hi @orlangur,

Actual result (*)
We have diff:

Please explain.

Result files after DI compile should be exactly the same when you running it from one commit with the same software versions, in my case - single docker image (including image hash) was used in all cases. But result is different.

using RecursiveDirectoryIterator object for generating list of all classes and all their arguments and it could return not sorted result.

This is perfectly valid. However, system behavior must not vary depending on order of generation.

It might not vary right now, but at some point it will because of some bug will be introduced. This part isn’t covered by 100% with tests.

https://reproducible-builds.org/ has nothing to do with this issue: it's all about no backdoors in compilers.

Directly - yes, it’s not related, but if we’ll look from another angle - it’s related. you can see example from symfony framework- they’re using the same approach https://symfony.com/blog/new-in-symfony-reproducible-builds

Explained more simply in the case of Symfony: if you build the container and warm up the cache of the same unchanged application multiple times, the result should always be the same.

For magento we could say the same - if you run DI compile of the same unchanged application multiple times, the result should be the same.

@orlangur
Copy link
Contributor

But result is different.

Just provide the difference. No buzz :)

@ihor-sviziev
Copy link
Contributor Author

But result is different.

Just provide the difference. No buzz :)

Diff really huge, but just to understand - in build 1 we have

<?php return array (
  'arguments' => 
  array (
    'Aheadworks\\Giftcard\\Api\\Data\\AmountExtension' => 
    array (
      'data' => 
      array (
        '_v_' => 
        array (
        ),
      ),
    ),
    'Aheadworks\\Giftcard\\Api\\Data\\AmountExtensionFactory' => 
    array (
      'objectManager' => 
      array (
        '_i_' => 'Magento\\Framework\\ObjectManagerInterface',
      ),
      'instanceName' => 
      array (
        '_v_' => '\\Aheadworks\\Giftcard\\Api\\Data\\AmountExtension',
      ),
    ),
    'Aheadworks\\Giftcard\\Api\\Data\\AmountInterfaceFactory' => 
    array (
      'objectManager' => 
      array (
        '_i_' => 'Magento\\Framework\\ObjectManagerInterface',
      ),
      'instanceName' => 
      array (
        '_v_' => '\\Aheadworks\\Giftcard\\Api\\Data\\AmountInterface',
      ),
    ),
    'Aheadworks\\Giftcard\\Api\\Data\\CodeGenerationSettingsExtension' => 
    array (
      'data' => 
      array (
        '_v_' => 
        array (
        ),
      ),
    )
...

In build 2 we have following:

<?php return array (
  'arguments' => 
  array (
    'Aheadworks\\Giftcard\\Pricing\\Price\\ConfiguredPrice' => 
    array (
      'saleableItem' => 
      array (
        '_i_' => 'Magento\\Catalog\\Model\\Product\\Interceptor',
      ),
      'quantity' => 
      array (
        '_vn_' => true,
      ),
      'calculator' => 
      array (
        '_i_' => 'Magento\\Framework\\Pricing\\Adjustment\\Calculator',
      ),
      'priceCurrency' => 
      array (
        '_i_' => 'Magento\\Directory\\Model\\PriceCurrency',
      ),
      'item' => 
      array (
        '_vn_' => true,
      ),
      'configuredOptions' => 
      array (
        '_vn_' => true,
      ),
    ),
    'Aheadworks\\Giftcard\\Pricing\\Price\\FinalPrice' => 
    array (
      'saleableItem' => 
      array (
        '_i_' => 'Magento\\Framework\\Pricing\\SaleableInterface',
      ),
      'quantity' => 
      array (
        '_vn_' => true,
      ),
      'calculator' => 
      array (
        '_i_' => 'Magento\\Framework\\Pricing\\Adjustment\\Calculator',
      ),
      'priceCurrency' => 
      array (
        '_i_' => 'Magento\\Directory\\Model\\PriceCurrency',
      ),
    ),
    'Aheadworks\\Giftcard\\Helper\\Data' => 
    array (
      'context' => 
      array (
        '_i_' => 'Magento\\Framework\\App\\Helper\\Context',
      ),
    ),
    'Aheadworks\\Giftcard\\Observer\\ControllerActionPredispatchObserver' => 
    array (
      'session' => 
      array (
        '_i_' => 'Magento\\Backend\\Model\\Session\\Interceptor',
      ),
      'actionFlag' => 
      array (
        '_i_' => 'Magento\\Framework\\App\\ActionFlag',
      ),
      'url' => 
      array (
        '_i_' => 'Magento\\Backend\\Model\\Url\\Interceptor',
      ),
    ),
    'Aheadworks\\Giftcard\\Observer\\OrderCreationProcessDataObserver' => 
    array (
      'giftcardCartManagement' => 
      array (
        '_i_' => 'Aheadworks\\Giftcard\\Model\\Service\\GiftcardCartService',
      ),
      'messageManager' => 
      array (
        '_i_' => 'Magento\\Framework\\Message\\Manager',
      ),
      'escaper' => 
      array (
        '_i_' => 'Magento\\Framework\\Escaper',
      ),
    ),
    ...
    'Aheadworks\\Giftcard\\Api\\Data\\AmountExtensionFactory' => 
    array (
      'objectManager' => 
      array (
        '_i_' => 'Magento\\Framework\\ObjectManagerInterface',
      ),
      'instanceName' => 
      array (
        '_v_' => '\\Aheadworks\\Giftcard\\Api\\Data\\AmountExtension',
      ),
    ),
    'Aheadworks\\Giftcard\\Api\\Data\\OptionExtension' => 
    array (
      'data' => 
      array (
        '_v_' => 
        array (
        ),
      ),
    ),
    
    ...
    'Aheadworks\\Giftcard\\Api\\Data\\AmountExtension' => 
    array (
      'data' => 
      array (
        '_v_' => 
        array (
        ),
      ),
    ),

All classes are present in both files, no differences between the data, just sort order is different.

I just showed for 1 custom module, but it appears for all modules, including Magento core.

@orlangur
Copy link
Contributor

@ihor-sviziev I see, just some class list in order of collecting. Did it ever cause any issue?

My understanding is that we need to fix underlying bugs in case order matters in some cases but generally list sorting each time looks like an overkill and can slowdown significantly as well.

Is there a way to put files and directories in pre-determined order into filesystem maybe?

@ihor-sviziev
Copy link
Contributor Author

ihor-sviziev commented Jun 24, 2019

@ihor-sviziev I see, just some class list in order of collecting. Did it ever cause any issue?

My understanding is that we need to fix underlying bugs in case order matters in some cases but generally list sorting each time looks like an overkill and can slowdown significantly as well.

Is there a way to put files and directories in pre-determined order into filesystem maybe?

I’m not sure if it ever created issues there, but you even can’t test it because result even on single machine might be different now, you can’t say that this version is correct and this is not.
Look at my PR - it adds one very simple sorting when we already get all paths to the modules. During my tests it’s not slowed down DI compile step, so it’s not a big deal

There actually similar issues with static content deploy - you’re getting different content of files js-translation.json and different content of js bundle files, but I still didn’t get to it.

@orlangur
Copy link
Contributor

Sorry, my bad, if we just sort paths and not full list of files sorting may be negligible. Will double check.

I’m not sure if it ever created issues there, but you even can’t test it because result even on single machine might be different now, you can’t say that this version is correct and this is not.

I meant real bugs caused by nondeterministic order of items in list.

There actually similar issues with static content deploy - you’re getting different content of files js-translation.json and different content of js bundle files, but I still didn’t get to it.

Does Symfony enforce SB with some build?

@ihor-sviziev
Copy link
Contributor Author

ihor-sviziev commented Jun 24, 2019

I meant real bugs caused by nondeterministic order of items in list.

I didn't saw any real bugs caused by this issue. I think just because this issue is not persistent.

Does Symfony enforce SB with some build?

I reviewed all PRs linked to symfony/symfony#25958 - found only one check - https://github.com/symfony/symfony/pull/26176/files#diff-f410485b2fdabe90d3c911496fbe9ea9R64
Not sure if we can do similar check

Actually test also could be like this:
https://github.com/lstrojny/symfony-reproducible-builds/blob/master/test-reproducible-build.sh

@ihor-sviziev
Copy link
Contributor Author

Hi @orlangur,
Any updates?

@orlangur
Copy link
Contributor

@ihor-sviziev sorry for delay, will post an update this week.

@m2-assistant
Copy link

m2-assistant bot commented Aug 23, 2019

Hi @engcom-Charlie. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

    DetailsIf the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

  • 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 4. Verify that the issue is reproducible on 2.3-develop branch

    Details- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 5. Add label Issue: Confirmed once verification is complete.

  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-Charlie engcom-Charlie added Component: Filesystem Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Aug 23, 2019
@ghost ghost unassigned engcom-Charlie Aug 23, 2019
@magento-engcom-team
Copy link
Contributor

✅ Confirmed by @engcom-Charlie
Thank you for verifying the issue. Based on the provided information internal tickets MC-19609 were created

Issue Available: @engcom-Charlie, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Aug 23, 2019
@ihor-sviziev ihor-sviziev self-assigned this Aug 23, 2019
@m2-assistant
Copy link

m2-assistant bot commented Aug 23, 2019

Hi @ihor-sviziev. Thank you for working on this issue.
Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

  • 1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 2. Verify that the issue is reproducible on 2.3-develop branch

    Details- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 3. If the issue is not relevant or is not reproducible any more, feel free to close it.


@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev. Thank you for your report.
The issue has been fixed in #23325 by @ihor-sviziev in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.4 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Aug 27, 2019
@sidolov
Copy link
Contributor

sidolov commented Sep 23, 2021

Hi @ihor-sviziev. Thank you for your report.
The issue has been fixed in #33903 by @ihor-sviziev in 2.4-develop branch
Related commit(s):

The fix will be available with the upcoming 2.4.4 release.

@sidolov sidolov added the Fixed in 2.4.x The issue has been fixed in 2.4-develop branch label Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Filesystem Fixed in 2.3.x The issue has been fixed in 2.3 release line Fixed in 2.4.x The issue has been fixed in 2.4-develop branch Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

5 participants