Skip to content
Snippets Groups Projects

Closes #3476189

Closed mondrake requested to merge issue/drupal-3476189:3476189-sudo-e into 11.x
2 unresolved threads

Closes #3476189

Merge request reports

Approval is optional
Code Quality is loading
Test summary results are being parsed

Closed by Dave LongDave Long 5 months ago (Jan 2, 2025 6:57pm UTC)

Merge details

  • The changes were not merged into 11.x.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 489 487 // that was executed.
    490 488 $php = $php_env;
    491 489 }
    492 elseif ($sudo = getenv('SUDO_COMMAND')) {
    490
    491 if ($sudo = getenv('SUDO_COMMAND')) {
    493 492 // 'SUDO_COMMAND' is an environment variable set by the sudo program.
    494 // Extract only the PHP interpreter, not the rest of the command.
    495 [$php] = explode(' ', $sudo, 2);
    493 // This will be set if the script is run directly by sudo or if the
    494 // script is run under a shell started by sudo.
    495 if (str_contains($sudo, basename(__FILE__))) {
    496 // This script may have been directly run by sudo. $php may have the
    497 // path to sudo from getenv('_') if run with the -E option.
    498 // Extract what may be the PHP interpreter.
    499 [$php] = explode(' ', $sudo, 2);;
  • 101 101 .run-tests: &run-tests
    102 102 script:
    103 103 # Need to pass this along directly.
    104 - sudo MINK_DRIVER_ARGS_WEBDRIVER="$MINK_DRIVER_ARGS_WEBDRIVER" -u www-data php ./core/scripts/run-tests.sh --color --keep-results --types "$TESTSUITE" --concurrency "$CONCURRENCY" --repeat "1" --sqlite "./sites/default/files/tests.sqlite" --dburl $SIMPLETEST_DB --url $SIMPLETEST_BASE_URL --verbose --non-html --all --ci-parallel-node-index $CI_PARALLEL_NODE_INDEX --ci-parallel-node-total $CI_PARALLEL_NODE_TOTAL
    104 - sudo -u www-data -E HOME=/var/www php ./core/scripts/run-tests.sh --color --keep-results --types "$TESTSUITE" --concurrency "$CONCURRENCY" --repeat "1" --sqlite "./sites/default/files/tests.sqlite" --dburl $SIMPLETEST_DB --url $SIMPLETEST_BASE_URL --verbose --non-html --all --ci-parallel-node-index $CI_PARALLEL_NODE_INDEX --ci-parallel-node-total $CI_PARALLEL_NODE_TOTAL
    • Why do we need the HOME variable?

    • See: https://git.drupalcode.org/issue/drupal-3476189/-/jobs/2835178

      Some of the scripts execute processes (such as git tests) that naturally try to reference files in $HOME.

      As we use -E $HOME retains /root which the www-data user does not have permission to access.

      Sudo without -E normally replaces the $HOME variable.

    • Thanks for the explanation.

    • Can we specify this once in the environment variables section and then -E will just pass it along - saving us from defining it each time (and possibly missing it in the future)?

    • Author Contributor

      Well that sets HOME for the www-data user, but HOME for the rest of the script should remain the one of the user running the script, no?

      Unless we add a WWW-DATA-HOME variable set to /var/www in the job setup, and then we do sudo -u www-data -E HOME=$WWW-DATA-HOME php ./core/scripts/run-tests.sh ...

      But that seems overkill to me.

    • As an alternative to HOME=/var/www we can use sudo -H parameter, that does the same, but looks better:

      Suggested change
      104 - sudo -u www-data -E HOME=/var/www php ./core/scripts/run-tests.sh --color --keep-results --types "$TESTSUITE" --concurrency "$CONCURRENCY" --repeat "1" --sqlite "./sites/default/files/tests.sqlite" --dburl $SIMPLETEST_DB --url $SIMPLETEST_BASE_URL --verbose --non-html --all --ci-parallel-node-index $CI_PARALLEL_NODE_INDEX --ci-parallel-node-total $CI_PARALLEL_NODE_TOTAL
      104 - sudo -u www-data -H -E php ./core/scripts/run-tests.sh --color --keep-results --types "$TESTSUITE" --concurrency "$CONCURRENCY" --repeat "1" --sqlite "./sites/default/files/tests.sqlite" --dburl $SIMPLETEST_DB --url $SIMPLETEST_BASE_URL --verbose --non-html --all --ci-parallel-node-index $CI_PARALLEL_NODE_INDEX --ci-parallel-node-total $CI_PARALLEL_NODE_TOTAL

      If it's okay, I'll make a commit with these changes here, and in other places.

    • Alexey Korepov changed this line in version 12 of the diff

      changed this line in version 12 of the diff

    • So, I applied the sudo -E -H approach and it works well, let's close this thread then?

    • Please register or sign in to reply
  • Conrad Lara added 1 commit

    added 1 commit

    • 39ee8f19 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Fran Garcia-Linares
  • mondrake added 1 commit

    added 1 commit

    • 51a4069d - Update file PerformanceTestTrait.php

    Compare with previous version

  • mondrake added 1 commit

    added 1 commit

    • 605bd3a0 - Revert "Update file PerformanceTestTrait.php"

    Compare with previous version

  • mondrake added 31 commits

    added 31 commits

    • 605bd3a0...9ba6f744 - 23 commits from branch project:11.x
    • 22a77155 - one
    • 7139cfa7 - Make run-tests.sh php path detection more robust (sudo -E) and output the path...
    • 14526bde - spelling error on last commit.
    • 992c6442 - Pass www-data home to scripts instead of root's home.
    • 7b354210 - Update file .gitlab-ci.yml
    • 84a9701b - Apply 1 suggestion(s) to 1 file(s)
    • 6afe3e09 - Update file PerformanceTestTrait.php
    • 2ad8556d - Revert "Update file PerformanceTestTrait.php"

    Compare with previous version

  • mondrake added 51 commits

    added 51 commits

    • 2ad8556d...4548e070 - 43 commits from branch project:11.x
    • 84f26c13 - one
    • dfaf348d - Make run-tests.sh php path detection more robust (sudo -E) and output the path...
    • 4beb8f23 - spelling error on last commit.
    • 7a6a75a5 - Pass www-data home to scripts instead of root's home.
    • 6cef6748 - Update file .gitlab-ci.yml
    • 8d3d9136 - Apply 1 suggestion(s) to 1 file(s)
    • 24f92d11 - Update file PerformanceTestTrait.php
    • 3462843f - Revert "Update file PerformanceTestTrait.php"

    Compare with previous version

  • mondrake added 88 commits

    added 88 commits

    • 3462843f...201c8394 - 78 commits from branch project:11.x
    • 55c4b225 - one
    • 2a6334d6 - Make run-tests.sh php path detection more robust (sudo -E) and output the path...
    • e91eb8fd - spelling error on last commit.
    • df2d8311 - Pass www-data home to scripts instead of root's home.
    • b513c4ca - Update file .gitlab-ci.yml
    • 39ee8f19 - Apply 1 suggestion(s) to 1 file(s)
    • 51a4069d - Update file PerformanceTestTrait.php
    • 605bd3a0 - Revert "Update file PerformanceTestTrait.php"
    • fc025427 - Merge branch '11.x' into 3476189-sudo-e
    • 13f32c75 - Merge branch '3476189-sudo-e' of git.drupal.org:issue/drupal-3476189 into 3476189-sudo-e

    Compare with previous version

  • added 1 commit

    • 66bdd7a9 - Replace `HOME=/var/www` to `-H` sudo option

    Compare with previous version

  • mondrake added 33 commits

    added 33 commits

    Compare with previous version

  • mondrake added 31 commits

    added 31 commits

    Compare with previous version

  • mondrake added 30 commits

    added 30 commits

    Compare with previous version

  • mondrake added 37 commits

    added 37 commits

    Compare with previous version

  • mondrake added 1 commit

    added 1 commit

    • f85bcb4b - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • closed

  • Please register or sign in to reply
    Loading