Skip to content

Conversation

@offbyone
Copy link
Contributor

@offbyone offbyone commented Mar 7, 2025

  • include the ports generated for other processes in each process definition
  • Add OVERMIND_PROCESS_<name>_PORT settings to each process wrapper script
  • add docs for the feature

Implements #189

…processes

- include the ports generated for other processes in each process
  definition
- Add `OVERMIND_PROCESS_<name>_PORT` settings to each process wrapper
  script
- add docs for the feature
@offbyone offbyone changed the title Add a feature to provide each process the generated ports from other … Add a feature to provide each process the generated ports from other processes Mar 7, 2025
@DarthSim
Copy link
Owner

Hey @offbyone!

Thanks for your contribution!

Here are a few things I would change before merging this:

  1. I don't think we need a flag that disables this behavior. I don't see a case where this would be required, and I don't want to occupy a short flag name.

  2. We don't need procfileEntry.OtherProcessPorts field since we already have entries list. We can pass it to createScriptFile.

  3. Process names may contain symbols not allowed in environment variable names. When building environment variable names, we should replace all non-alphanumeric symbols with an underscore.

@offbyone
Copy link
Contributor Author

I'm happy to make all of those changes. I erred on the side of over-gating the feature but it should be easy to simplify.

@offbyone
Copy link
Contributor Author

offbyone commented Mar 11, 2025

Looking at my own shell and the Open Group's docs, it looks like = is the only character not permitted in an environment variable name:

$ env FAKE_😂=something env | grep FAKE
FAKE_😂=something
$ bash
bash-5.2$ env FAKE_😂=something env | grep FAKE
FAKE_😂=something

I was not able to make this fail on any machine I have access to.

I didn't see any way to make a procfile with a process whose name contained = either. I put in the safeguard nonetheless because it's cheap and only done once per run.

- drop the extra command line flag and gate
- add a bit of extra safety to the port environment variable name
Construct the other service port list as the script is generated
@DarthSim
Copy link
Owner

Looking at my own shell and the Open Group's docs, it looks like = is the only character not permitted in an environment variable name

The behavior differs from shell to shell. For example, Fish allows variable names to contain only alphanumeric symbols and underscores. I would rather not take risks here; it's better to be safe than sorry.

@offbyone
Copy link
Contributor Author

My example actually was in fish shell. Here's further indications that fish handles environment (as opposed to shell) variables with emoji just fine:

$ set var_with_😂 "foo"
set: var_with_😂: invalid variable name. See `help identifiers`

(Type 'help set' for related documentation)
$ env ENV_VAR_WITH_😂=foo fish
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish
$ env | grep ENV_VAR_
ENV_VAR_WITH_😂=foo

@offbyone
Copy link
Contributor Author

(that said, if you want me to, I still can carve those out, I don't mind, but I don't think it's an issue here)

@DarthSim
Copy link
Owner

I don't worry much about emojis. What I worry about is symbols that a shell can wrongly interpret. For example, Fish doesn't allow using # in variable names:

set: LOREM_IPSUM#DOLOR: invalid variable name. See help identifiers

Here's what Fish's docs say:

A variable name cannot be empty. It can contain only letters, digits, and underscores. It may begin and end with any of those characters.

So, the fact that it allows emojis in variable names is rather a bug than a feature.

As you may have noticed, I don't have much time to maintain Overmind because I am overloaded with https://imgproxy.net. (Also, because I believe that Overmind is already ideal and doesn't need any improvements 😅) Thus, I don't want to push my luck here and hope I won't need to fix this a month later.

@offbyone
Copy link
Contributor Author

Sounds good. I'll strip them down to [A-Za-z0-9_]

@DarthSim DarthSim merged commit 7920e86 into DarthSim:master Mar 12, 2025
5 checks passed
@DarthSim
Copy link
Owner

Thanks again for your contribution!

@dobesv
Copy link

dobesv commented Dec 11, 2025

Was this feature released? It doesn't seem to be working for me, and the release notes for releases around this time do not seem to mention it.

@rolodato
Copy link

It was not released. The code from the latest release does not have this feature: https://github.com/DarthSim/overmind/blob/v2.5.1/start/command.go#L88

Example:

➜ overmind --version
Overmind version 2.5.1
➜ cat Procfile
p1: echo port is $PORT; sleep 5
p2: echo p1 port is $OVERMIND_PROCESS_p1_PORT
➜ overmind start
system | Tmux socket name: overmind-sandbox-Srjr-mHisG-iQUFrpDh1C
system | Tmux session ID: sandbox
system | Listening at ./.overmind.sock
p2     | Started with pid 44907...
p1     | Started with pid 44906...
p2     | p1 port is
p2     | Exited with code 0
p1     | Interrupting...
p1     | Exited with code 0

@DarthSim This is a really useful feature and would love to get it released. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants