Repository navigation
Conversation
meta-cla
Bot
added
the
CLA Signed
lazerg
changed the title
facebook-github-tools
Bot
added
the
Shared with Meta
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D127824392. |
cipolleschi
left a comment
cipolleschi
left a comment
There was a problem hiding this comment.
Thanks for the fix. We discovered an edge case. Could you address it, please?
| file_manager.write("#{file_path}.local", local_env.gsub(/^export NODE_BINARY=.+$/) { "export NODE_BINARY=#{node_binary}" }) | ||
| else | ||
| system("echo 'export NODE_BINARY=#{node_binary}' > #{file_path}.local") | ||
| end |
There was a problem hiding this comment.
Empty/malformed NODE_BINARY value treated as valid, leaving broken file unrepaired
local_node_binary_exists? returns true when the captured NODE_BINARY is nil, but an empty line export NODE_BINARY= fails the (.+) capture and produces nil — so a file with an empty/unset NODE_BINARY is considered valid and left untouched. This is exactly the broken-build state the diff is meant to repair, so the build continues to fail.
There was a problem hiding this comment.
Good catch, fixed in c951f79. An empty value, including "", now counts as broken and gets replaced on pod install. I added a test for it.
Visual regressionNo visual differences at android: no capture |
Summary:
pod installonly writesios/.xcode.env.localwhen the file is missing. The Xcode script phases source it after.xcode.env, so its absoluteNODE_BINARYwins. Once a version manager removes that node (nvm installing a newer one and uninstalling the old), every build fails in the first script phase that runs node, and runningpod installagain doesn't help because the file already exists.create_xcode_env_if_missingnow checks theNODE_BINARYthe file exports. If it's an absolute path that no longer exists, that line is replaced with the current node and the rest of the file is kept. Files pointing at an existing node, or using something other than an absolute path (like$(command -v node)), are left alone.Fixes #59538
Changelog:
[IOS] [FIXED] - Update
.xcode.env.localonpod installwhen the node it points to was removedTest Plan:
Added
test_createXcodeEnvIfMissing_whenLocalNodeIsMissing_updatesNodeBinarytoutils-test.rband updated the "present" test to read a local file whose node exists.ruby -Itest cocoapods/__tests__/utils-test.rbpasses, excepttest_SetGCCPreprocessorDefinitionForHermes_itSetsThePreprocessorForDebug, which fails the same way onmainlocally.Also ran
create_xcode_env_if_missingagainst a real folder: a.xcode.env.localpointing at a removed nvm node gets itsNODE_BINARYline updated, while existing paths, escaped-space paths and$(command -v node)are left alone.