Repository navigation
Give every 51Degrees environment variable the underscore name - #100
Closed
jwrosewell wants to merge 1 commit into
Closed
jwrosewell wants to merge 1 commit into
jwrosewell wants to merge 1 commit into
Conversation
A POSIX shell cannot set a name that starts with a digit, so every name becomes _51DEGREES_<NAME> and the digit form is not read anywhere. The duplicate constants that held the two spellings of the resource key are reduced to one, and the guard test fails on any name in the digit form.
This was referenced Nov 11, 2026
Contributor
Author
|
Closed in favour of #105, which makes the same rename to the 51DEGREES names more fully. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
A POSIX shell rejects an environment variable name that starts with a digit,
so several of the names this repository documented could not be set the way it
told a developer to set them. Measured in bash:
That covered
51DEGREES_RESOURCE_KEY,51DEGREES_DD_PATHand51DEGREES_IPI_PATH. The repository had also grown a second,underscore-prefixed name beside the resource key one, so two constants held
what was meant to be the same thing, and the on-premise web example's pipeline
configuration carried the shell default form
${51DEGREES_DD_PATH:-${TestDataFile:-...}}, which a shell can never fill in.What the specification says
POSIX.1-2024 (IEEE Std 1003.1-2024),
Base Definitions section 8.1, Environment Variable Definition:
Base Definitions 3.216, Name,
which is what
exportand a variable assignment take:So a process may receive a digit-initial name, but a shell can never set one.
The decision
James Rosewell decided, in his words, "Just use the version that works
everywhere all the time with no fallback." Every name becomes
_51DEGREES_<NAME>, and the name without the leading underscore is not readanywhere. This follows
51degrees-rust#378 and
51degrees-dotnet#131,
which made the same change in those languages, so the names match across the
three repositories. All three use
_51DEGREES_RESOURCE_KEY,_51DEGREES_DD_PATHand_51DEGREES_IPI_PATH.What changed
Twenty-two places in fifteen files, which the guard test below counted.
examples/ip-intelligence/shared/.../KeyHelper.java
held
RESOURCE_KEY_ENV_VARas"51DEGREES_RESOURCE_KEY"whileSHARED_RESOURCE_KEY_NAMESalready held"_51DEGREES_RESOURCE_KEY", soafter the rename the two would have been the same string. The constant, its
entry in the list
requireTestResourceKeypasses, and its documentation aredeleted, and
getOrSetResourceKeyreads the first ofSHARED_RESOURCE_KEY_NAMESinstead. That leaves the file the same shape asthe Device Detection
KeyHelper.java,
which never had the second constant.
ip-intelligence.shared/.../FileUtils.java
and
examples/.../DataFileHelper.java
each hold one constant with no duplicate beside it, so each is a plain
rename of the value. Every caller keeps working through the constant.
51Degrees-OnPrem.xml
now reads
${_51DEGREES_DD_PATH:-${TestDataFile:-...}}, which a reader canactually set with
export. Its comment, the example's class documentation inGettingStartedWebOnPrem.java
and
OnPremDataFileOptionTest.java,
which checks the order the example picks its data file in, move with it.
ci/run-integration-tests.ps1,
which passes the data file path to the on-premise Selenium contract example.
The workflows under
.github/workflows
already used the underscore names, so none of them changed.
AGENTS.md,
examples/device-detection/README.md,
examples/ip-intelligence/README.md
and
ip-intelligence/README.md.
OfflineProcessing.java,
CountryTranslationIntegrationTest.java,
which reads the name directly rather than through a constant, and the IP
Intelligence
KeyHelperTest.java,
whose expected message no longer names the deleted constant's value.
What stays, and why
The legacy names that do not start with a digit stay, because a shell can
set them and nothing about them is broken.
51degrees-rust#378 and
51degrees-dotnet#131
kept their equivalents for the same reason. In this repository those are
TestResourceKeyandSuperResourceKeyfor the resource key, andTestDataFilefor the data file paths. Each is still checked after the_51DEGREES_*name. Removing them is a separate decision, because it wouldbreak callers who have set a name that works today.
Nothing with a
51D_prefix was touched, because those are cookie prefixesand evidence keys in the Share Usage, geolocation and 51Did code rather than
environment variables, and the
51Degrees-...names are file names. The guardtest below matches only the
51DEGREES_form, so it leaves both alone.The guard test
pipeline/pipeline.common/src/test/java/fiftyone/common/testhelpers/EnvironmentVariableNamesTest.java
is a port of
the Rust test.
It walks up from the working directory to the folder that holds
.git, liststhe files with
git ls-files -zso build output and the submodules are leftout, skips a tracked path that is not a file, and fails on any name that
starts with the digit, naming the file, the line and the name. It builds the
text it looks for as
"5" + "1DEGREES_"so the test file does not matchitself.
theNameMatcherFindsOnlyNamesThatStartWithTheDigitproves the matcherfinds
51DEGREES_DD_PATHand does not match_51DEGREES_DD_PATHorX51DEGREES_DD_PATH.It sits in
pipeline.common,
whose only dependencies are slf4j, logback and junit, so it needs no data file,
no resource key and no native library and the normal unit test run executes it
on every platform. It reads bytes rather than text, so a file in any encoding
is scanned and a file that is not text cannot throw.
Runs
Red, on
f6784e13,
which is
rcat d39b6af with only the guard test and the temporary workflowadded and no rename:
failing on both Java 8 and Java 21 with
noFileNamesAVariableThatStartsWithADigit ... FAILURE!and"22 names start with a digit, which a POSIX shell cannot set, so each must
start with an underscore".
theNameMatcherFindsOnlyNamesThatStartWithTheDigitpassed in the same run, so the matcher was working and the 22 were real. The
job reports failure, because the temporary workflow does not set
-Dmaven.test.failure.ignore.Green, on
ca83cdb2,
which is this branch's head with the same temporary workflow on top:
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0on both Java 8 and Java 21,so both the scan and the matcher test pass with the rename in place.
Full build:
FULL_RESULT
macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.
What this changes for callers
These names stop being read. Anyone who set one must set the
_51DEGREES_name instead, and this belongs in the release note.51DEGREES_RESOURCE_KEY_51DEGREES_RESOURCE_KEY51DEGREES_DD_PATH_51DEGREES_DD_PATH51DEGREES_IPI_PATH_51DEGREES_IPI_PATHNo published Maven artifact reads any of them. Every one is in the examples,
the test helpers, a CI script and the documentation, so no consumer of a
published package is affected. A developer or a CI definition that set an old
name gets the default behaviour until the name is changed, which for a data
file path means the example searches for the file and downloads the free one
instead, and for the resource key means the cloud example reports that no key
is set.
The non-digit legacy names listed above are unaffected and still work, so
anyone setting
TestResourceKey,SuperResourceKeyorTestDataFileneedsto do nothing.
One Java constant changed its name as well as its value, which matters to
anyone who compiled against the examples.
KeyHelper.RESOURCE_KEY_ENV_VARinfiftyone.ipintelligence.examples.sharedno longer exists, andKeyHelper.SHARED_RESOURCE_KEY_NAMES.get(0)holds the aligned name in itsplace. That class is in an example module rather than a published engine, so
nothing on Maven Central refers to it.
What remains
cxxsubmodule's own C and C++ examples read the data file paths underthe old names. That is changed in that repository, and this one takes it
with the next submodule update. Nothing here runs those examples, and the
guard test leaves a submodule out because
git ls-filesreports it as adirectory rather than a file.
tmp/envnames-redandtmp/envnames-greenbranches, and the temporaryworkflow file on each, exist only to show the red and green runs above. Both
are deleted once this is reviewed.