Make WordPress Core

Opened 4 years ago

Closed 6 months ago

Last modified 6 months ago

#52668 closed enhancement (fixed)

Make it easier to override the built-in docker environment's config

Reported by: timothyblynjacobs's profile TimothyBlynJacobs Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

Core currently ships with a .env file to provide a default configuration to the built-in Docker environment. If you want to override some of these variables you can setup values as environment variables before invoking the npm run env:* commands.

It can be a somewhat frustrating DUX if you need to consistently override these values, and not just once for testing some specific configuration. For instance if you need to use mariadb for ARM support, or making sure the correct port is used.

One possibility to make this more permanent is to modify the .env file itself, but then you need to remember to exclude changes to that file when committing which is a bit annoying.

There is the ability to create a docker-compose override file. But IMO that is a bit of overkill if just using the existing environment variable configuration would be sufficient.

One way I think we could solve this would be to change the .env file to .env.example and then in a post install routine copy the file from .env.example to .env if a .env file does not already exist. That way, if a user wants to they can customize that .env file without any fear that a future pull will overwrite their changes or that their changes would be accidentally included in a commit.

Attachments (1)

52668.diff (480 bytes) - added by h71 23 months ago.

Download all attachments as: .zip

Change History (19)

#1 @johnbillion
4 years ago

  • Keywords needs-patch added
  • Version trunk deleted

#2 @afragen
4 years ago

Just to chime in. For Macs you can determine the chip architecture using uname -m.

Intel based Macs

$ uname -m
x86_64

M1 based Mac

$ uname -m
arm64

It might be useful in a setup script.

This ticket was mentioned in PR #4593 on WordPress/wordpress-develop by @h71.


23 months ago
#3

  • Keywords has-patch added; needs-patch removed

Trac ticket: https://core.trac.wordpress.org/ticket/52668

Review notes:
A separate commit is present for adding EOF to .gitignore and it is not directly related to 52668 but it is very small and good to have, without side effects.

@h71
23 months ago

#4 @h71
23 months ago

The provided patch renames .env to .env.example without any changes in the file content.
It also adds .env to .gitignore to make sure it won't be committed to the repository again.
Also, an empty line is added to the .gitignore file.

#5 @desrosj
7 months ago

Talking with @TimothyBlynJacobs at WCUS Contributor Day. This seems like a good improvement. I think the best place to copy .env.default if one does not yet exist would be in the start.js script before running dotenvExpand.expand().

This ticket was mentioned in PR #7380 on WordPress/wordpress-develop by @TimothyBlynJacobs.


7 months ago
#6

The .env file allows for configuring how the WordPress Local environment should be configured. However, because the file is version controlled, developers must be careful not to commit their modifications.

This commit renames the .env file to be .env.example. During env start, the .env.example file is copied to .env if it does not exist. This allows for contributors to continue using the project without thinking about .env and to make changes when needed. This brings WordPress Core into the dotenv project guidelines.

Props johnbillion, afragen, h71.
Fixes #52668.

Trac ticket: https://core.trac.wordpress.org/ticket/52668

#7 @TimothyBlynJacobs
7 months ago

  • Owner set to TimothyBlynJacobs
  • Resolution set to fixed
  • Status changed from new to closed

In 59038:

Build Tools: Allow easier customization of the .env file.

The .env file allows for configuring how the WordPress Local environment should be configured. However, because the file is version controlled, developers must be careful not to commit their modifications.

This commit renames the .env file to be .env.example. During env start, the .env.example file is copied to .env if it does not exist. This allows for contributors to continue using the project without thinking about .env and to make changes when needed. This brings WordPress Core into the dotenv project guidelines.

Props johnbillion, afragen, h71, desrosj.
Fixes #52668.

@TimothyBlynJacobs commented on PR #7380:


7 months ago
#8

Merged in r59038.

#9 @afercia
7 months ago

@TimothyBlynJacobs @desrosj [59038] appears to break the start script on Windows, as test and cp aren't available commands.

#10 @desrosj
7 months ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from Awaiting Review to 6.7
  • Resolution fixed deleted
  • Status changed from closed to reopened

Thanks for flagging this, @afercia. Reopening to get the problem fixed.

I've opened a ticket for improving the Docker environment documentation for Windows contributors (#62196), and one to create a GitHub Actions workflow for testing changes to the environment (#62197).

The workflow suggested in #62197 should help us catch these things in the future. @afercia if you could, could you add any details about how you get the environment up and running successfully on Windows to those tickets? I'm afraid I don't have a Windows machine handy so I'm unable to test.

This ticket was mentioned in PR #7546 on WordPress/wordpress-develop by @desrosj.


7 months ago
#11

  • Keywords has-patch added; needs-patch removed

#12 @desrosj
7 months ago

I discussed with @Clorith a bit and I've opened a PR that uses fs instead of cp/test, which should allow for better cross-platform compatibility.

@afercia could you give that PR a try?

#13 @afercia
7 months ago

@afercia if you could, could you add any details about how you get the environment up and running successfully on Windows to those tickets? I'm afraid I don't have a Windows machine handy so I'm unable to test.

@desrosj I'm on macOS. The issue was reported by @poena who often works on Windows. @SergeyBiryukov could also help but I'm not sure he's around at the moment.

@desrosj commented on PR #7546:


6 months ago
#14

👋 @carolinan I added you as a reviewer because Andrea said you were the person having issues. Whenever you have a chance, could you please just give this PR a test to see if the issue is fixed?

@poena commented on PR #7546:


6 months ago
#15

I can confirm that the docker containers start without problems when I use the command npm run env:start
Windows 11 Pro OS Version 22631.4317

#16 @desrosj
6 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 59249:

Build/Test Tools: Change commands used for the copying .env.example file.

This switches from using the test/cp commands when copying the .env.example file to using node:fs. test and cp are not available on Windows machines.

This also adds the .env file to the svn:ignore list to prevent it from being committed accidentally.

Follow up to [59038].

Props afercia, Clorith, poena.
Fixes #52668.

#18 @desrosj
6 months ago

In 59251:

Build/Test Tools: Add input for PHPUnit test group.

This adds a phpunit-test-goups input to the reusable PHPUnit test workflow for added flexibility.

When passed, only the specified test groups are run.

Props jrf.
See #52668.

Note: See TracTickets for help on using tickets.