Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

During builds, prefix environment variables with USER_DEFINED_ instead of USER-DEFINED_ #161

Closed
philihp opened this issue Mar 8, 2019 · 25 comments
Assignees
Labels
build Related to App Center's Build service feature request New feature request

Comments

@philihp
Copy link

philihp commented Mar 8, 2019

Describe the solution you'd like

When building with custom environment variables, do not prefix them with a string that contains an invalid character, such as USER-DEFINED_

Describe alternatives you've considered

Instead, prefix it with the string USER_DEFINED_

Additional context
see MicrosoftDocs/appcenter-docs#488 (comment)
and https://unix.stackexchange.com/questions/23659/can-shell-variable-include-character/23714#23714

@philihp philihp added the feature request New feature request label Mar 8, 2019
@patniko patniko added the build Related to App Center's Build service label Mar 8, 2019
@tonyarnold
Copy link

This is critical, it's also not documented anywhere - I've just wasted half a day trying to get my builds to run by defining variables in App Center.

Why are these being prefixed in the first place? It seems unnecessary, and it means that I need to have specific setups just for App Center.

@dennispan
Copy link

Oops just noticed this Issue. To copy paste what I responded in the appcenter doc repo:

The environment variables configured in App Center Build are currently specifically designed to be used in places documented here: https://docs.microsoft.com/en-us/appcenter/build/custom/variables/#access-the-variables, where you can access them without the USER-DEFINED_ prefix. This is achieved by storing the variables with USER-DEFINED_ prefix, and then remove the prefix when we make them available at the places described in the doc.

Accessing the variables outside of those documented place are currently unsupported. Directly using USER-DEFINED_xxx really wasn't our intention. That said, I would definitely be open to hear what you are trying to solve (e.g. where you are trying to access the variables), and see what we can do.

@philihp
Copy link
Author

philihp commented May 8, 2019

@dennispan glad to see this is finally getting some attention. What is the point of adding the prefix?

The text description below "Environment Variables" might be the culprit here, as it could be misleading unenlightened users to think they can be used as environment variables during the build process.

@tonyarnold
Copy link

Yeah, I have to be honest - the way it's described on the website makes it sound super useful.

The way it's implemented: it's not. I want to be able to pass environment variables to xcodebuild and other tools in my build process in the format that they expect.

I get what the App Center team we're aiming for here, but it's pretty useless for wider use cases. Perhaps have a look at how other build and CI services implement this feature to get an idea of the community's expectations?

@dennispan
Copy link

@philihp @tonyarnold We’re going to look into this more. There was reason why we added the prefix at the time. But we may get around it with other approach. Will keep you updated.

At the moment, I doubt this is something will be top of the priority list. For a quick adjustment, we can potentially follow the suggestion to change dash to underscore in the name.

@tonyarnold
Copy link

Thanks for the update @dennispan. I'm disappointed to hear that you're going to continue to use the inaccurate description for this feature for now, but I do understand that you have competing priorities.

For the near-term, replacing the dash with an underscore makes it a valid environment variable again, which would be appreciated.

@yamov
Copy link

yamov commented Jun 2, 2019

For a quick adjustment, we can potentially follow the suggestion to change dash to underscore in the name.

Any news here?

@philihp philihp mentioned this issue Jun 2, 2019
18 tasks
@dennispan
Copy link

@yamov sorry I lost track on this. We've been focusing on May iteration plan and moving on to the June one. Let me check with the team whether this is a quick fix we can apply, or would involve larger effort. If former, we may be able to squeeze in a change soon. If latter, we will then have to look at iteration plan for the month after.

@yamov
Copy link

yamov commented Jun 4, 2019

@dennispan Thanks for the info. In any case would be nice to get an update here after you've discussed it with the team 🙏.

@dennispan
Copy link

@yamov We believe this should be a small change. So hopefully we can update this within a week or so. Still pending prioritization.

Long term I’d like to see if we can remove the prefix. The reason we had it was to prevent conflicts with pre-defined variables on the machine. But we may have better solution for that.

Worth noting that using the variable as “USER_DEFINED_xxxx” still won’t be something officially supported. It’s not how the current design of environment variables work. We may change the design and remove the prefix in the future, which could break you if you use them as “USER_DEFINED_xxxx”

@philihp
Copy link
Author

philihp commented Jun 6, 2019

@octopotato
Copy link

octopotato commented Jul 9, 2019

@philihp good news! I managed to find some time this week and made the change for environment variables from USER-DEFINED_XXXX to USER_DEFINED_XXXX. Just to reiterate over what Dennis said, this isn't something we're officially supporting and hopefully unblocks your scenario in the meantime while we explore a better approach for environment variables.

You'll need to re-save your branch configuration for it to update the environment variable notation used and new builds after that should use the underscore format.

@philihp
Copy link
Author

philihp commented Jul 9, 2019

cheers to you, good sir!

@octopotato
Copy link

Unfortunately we found a severe regression due to this change and were forced to revert :(. We likely won't be able to take a second look this week but hopefully will get some time next week to try and get this resolved. Sorry for the inconvenience this caused!

@octopotato octopotato reopened this Jul 15, 2019
@philihp
Copy link
Author

philihp commented Jul 23, 2019

@gfrileux
Copy link

gfrileux commented Aug 2, 2019

Hi all,

Just a two cent here, and maybe I'm missing something really obvious, but we worked around the issue by create a env.txt file in the post_clone phase.
We use a simple appcenter-post-clone.sh bash script to create a env.txt file. The file is built using a series of commands as shown below:
echo "export CODEPUSH_KEY=\"$CODEPUSH_KEY\"" >> env.txt
If you have 10 variables, you need to add one line per variable.

At any point of the build, if you need your ENV VARS in a bash script, you can just start with :

if [ -f ../env.txt ]
then
    source ../env.txt
fi

This will export all the env vars stored in your env.txt - thus accessing your ENV VARs using their "normal name"
Hope that helps !
Bit ugly, but works, pending changes from our friends at AppCenter

@evkhramkov
Copy link

@philihp @vinoa-team sorry for delay on that.
Could you please provide more details about your cases? Os and platform of your apps and which step require env variables during build would be helpful.
We have some limitations stoping us from changing prefix name, but we can look in extending number of build steps where variables passed directly without prefixes

@philihp
Copy link
Author

philihp commented Aug 8, 2019

We had two branches building, release and staging. During the build, it would load a CODEPUSH_KEY from an environment variable and bake this into the app

This avoids the complexities of Xcode build configurations, as suggested in https://github.com/microsoft/react-native-code-push/blob/master/docs/multi-deployment-testing-ios.md

Could you be more transparent about the limitation that requires a dash be included in the environment variable name?

@gfrileux
Copy link

gfrileux commented Aug 9, 2019

@philihp @vinoa-team sorry for delay on that.
Could you please provide more details about your cases? Os and platform of your apps and which step require env variables during build would be helpful.
We have some limitations stoping us from changing prefix name, but we can look in extending number of build steps where variables passed directly without prefixes

Hi - sure. We develop a React native app, available both on iOS and Android.
We use AppCenter to build on develop branch for testing purposes, and then on master for releases.
For runtime ENV VARS, it's pretty simple: we use an env.json file that stores our dev ENV VARS. When building on develop, we don't touch it, when building on master, we havea postclone script that takes all the ENV VARS available on the AppCenter config, and overwrite the env.json file. This works very well.
The complexity was more for accessing ENV VARS during the build phase. I see that on Android, ENV VARS are available now via a simple System.getenv() which is great ! So we can do something like:
buildConfigField "String", "CODEPUSH_KEY", '"' + System.getenv("CODEPUSH_KEY") + '"' in our gradle file (using


         buildToolsVersion = "28.0.3"
        minSdkVersion = 16
        compileSdkVersion = 28
        targetSdkVersion = 28
        supportLibVersion = "28.0.0"

at the moment.

However the same doesn't seem to be true for iOS. If you need an ENV VAR during build you either have to try and use $USER-DEFINED_CODEPUSH_KEY but this is not supported, could change, and I don't even know if it works with encrypted ENV VARS (something not mentioned much in this thread, and that we struggled with in recent past).
So as a result we used the system described in my previous post.
On iOS we build using xCode 10.1

Hope that helps ?

@evkhramkov
Copy link

evkhramkov commented Aug 12, 2019

@vinoa-team @philihp we released a fix to pass env variables without prefixes to build steps. Could you please try if it will work for your builds now? Please make sure to re-save branch configuration before queuing build.

Additional question regarding this statements:

During the build, it would load a CODEPUSH_KEY from an environment variable and bake this into the app
For runtime ENV VARS, it's pretty simple: we use an env.json file that stores our dev ENV VARS.

Do you have any specific library or build configuration to bake vars in bundle?

@philihp
Copy link
Author

philihp commented Aug 13, 2019

Word, this is great, thanks!

giphy

Below is the Xcode build step I added which pulls the key that Appcenter sends it as an environment variable based on the branch it's building.

/usr/libexec/PlistBuddy -c \
  "Set :CodePushDeploymentKey ${CODEPUSH_KEY}" \
  "$PRODUCT_SETTINGS_PATH"

Screen Shot 2019-08-13 at 1 20 55 AM

@gfrileux
Copy link

Hi @evkhramkov - thanks for the update and for the release. I just tested it, and it works perfectly ! I can access my own custom ENV VARS within the build stage on iOS now (I think Android was already supported since a little while ago ? It works well too)

With regards to accessing ENV VARS in runtime code, we don't really use a library. We have an env.json file which contains the ENV VARS. It contains by default all the dev values, and we have a post-clone script that, when building on master, takes the custom ENV VARS from AppCenter, and overwrites the values set in the env.json. This means that when the app is built and run, any import of a variable from that env.json will contain the correct values.
There's most probably a dotenv library that could sort of do the same thing for Rect Native, but we've found this system to work ok, albeit you have to make sure your post-clone script and your AppCenter branch config have to be well aligned but that's fairly easy to maintain.

Am guessing the issue can be closed now ? :-)

@evkhramkov
Copy link

@vinoa-team awesome, thanks for confirmation and addition details!

@philihp could you please confirm that it works for you as well?

@dennispan
Copy link

Closing this. Feel free to re-open it if the fix is not working.

@godboutj
Copy link

I was trying to add some precompile define for the build process. (#define and #if #elif...), I have no need for runtime env file there are already managed with the build config properly. How can we add some precompile define? or msbuild /D flag if you prefer? this is very lacking from the build process? why not just give us extra arg to the msbuild? Feel like I should be building with Jenkins and only use AppCenter for distributing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to App Center's Build service feature request New feature request
Projects
None yet
Development

No branches or pull requests

10 participants