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.

Sign up for GitHub

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

Jump to bottom

Command to generate the travis.yml file produces faulty output #19941

Closed
justinvelluppillai opened this issue Nov 2, 2022 · 7 comments
Closed

Command to generate the travis.yml file produces faulty output #19941

justinvelluppillai opened this issue Nov 2, 2022 · 7 comments
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Milestone
4.13.0

Comments

@justinvelluppillai
Copy link
Contributor

justinvelluppillai commented Nov 2, 2022 •
edited

The generate:travis-yml command is generating yaml files with broken newlines. I tested in PHP 8.1 and 7.4 with the same results. It could be worth testing this works in PHP 7.2 as this may have been the last PHP version it was successfully run in.

Ideally we don't update the travis.yml files manually, but can use the command.

Expected Behavior

Properly formatted yaml files for --core and --plugin=<PluginName>

Current Behavior

Some newlines are missing (not all new lines)

Possible Solution

Steps to Reproduce (for Bugs)

  1. Run the command ./console generate:travis-yml --core or ./console generate:travis-yml --plugin=TagManager
  2. Compare the output travis.yml file with the existing one and you'll see it has some newlines missing, breaking the formatting

Context

Your Environment

  • Matomo Version:
  • PHP Version:
  • Server Operating System:
  • Additionally installed plugins:
  • Browser:
  • Operating System:
@justinvelluppillai justinvelluppillai added Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. Regression Indicates a feature used to work in a certain way but it no longer does even though it should. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Nov 2, 2022
@mattab mattab added this to the 4.12.4 milestone Nov 2, 2022
@peterhashair
Copy link
Contributor

peterhashair commented Nov 3, 2022 •
edited

It seems like when twig renders the template missing some of the\n line breaks, caused by php8 or a higher version. Once I run composer update update twig/twig from 1.18 to v1.44.7, generated format starts to work. Do we consider the update? Which is that PR matomo-org/travis-scripts#81

@justinvelluppillai justinvelluppillai modified the milestones: 4.12.4, 4.12.5 Nov 4, 2022
@peterhashair
Copy link
Contributor

PR matomo-org/travis-scripts#81 is merged, but seems like we still need to do a composer install on the ./tests/travis/generator/ to update vendor, not sure if should we manually do it, or if there or there is a GitHub step for auto-update.

@sgiehl
Copy link
Member

sgiehl commented Nov 7, 2022

I didn't have a look in detail here, but simply merging this PR might possibly even create more trouble that we currently have. On travis we try to auto generate the travis.yml. That is always done in the first job (not for PRs I think). This jobs are running with PHP 7.2 in most cases. So if updating twig changes the result also for PHP 7.2, this might mean that those jobs will start failing now and we need to update all .travis.yml files in all plugin repos.

@peterhashair
Copy link
Contributor

@sgiehl right maybe is out of scale for this issue, but since PHP 7.2 is deprecated. Do we want to update the test from 7.2 to 7.4? I test 7.4 worked as expected.

@sgiehl
Copy link
Member

sgiehl commented Nov 8, 2022

No. We always need to run our tests against the lowest PHP version we are supporting. Otherwise we might miss incompatibilities there...

@sgiehl
Copy link
Member

sgiehl commented Nov 15, 2022

Just a note: On PHP 7.2 the output is completely as expected.

@peterhashair
Copy link
Contributor

close this issue now, test PHP 7.2 works

@elabuwa elabuwa added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Nov 22, 2022
@justinvelluppillai justinvelluppillai modified the milestones: 4.12.5, 4.13.0 Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.
Projects
None yet
Milestone
4.13.0
Development

No branches or pull requests
5 participants
@mattab @justinvelluppillai @sgiehl @elabuwa @peterhashair