Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

"Fix" Issue #841 #846

Closed

Conversation

ntakouris
Copy link
Contributor

I want to make sure if this should be the proper way to set the development environment for the build, in order to procceed with testing & possible mistake corrections.

A new environment flag is registered and defaulted to "development".
Then, on bin.go, its passed to envy (not persisted to the underlying environment), in order to procceed to exec.go without changing function signatures.

The value is retrieved from envy and appended in the Env slice of exec.Cmd so it can be used instead of the default one on the actual command execution step.

#841

@markbates markbates changed the base branch from master to development January 11, 2018 19:16
@markbates
Copy link
Member

Thanks for looking into, unfortunately it doesn't solve the issue of #841. The behind that ticket is the binary that built from running buffalo build can be set to run in an environment other than development. This means the binary needs to get the environment "burnt" into it. This PR simply sets the requested environment of the buffalo build command, and unfortunately, not the finish binary. Does that make sense?

Also, this line:

cmd.Env = envy.Environ()

breaks Windows, which is why it was removed and caused the problem of #845. When implemented #841 won't fix #845 unfortunately.

@ntakouris ntakouris closed this Jan 11, 2018
@markbates
Copy link
Member

The good news is, you're very close to the actual implementation. :) I was thinking that adding a line to the main.go file that gets generated as part of the build process could contain something like:

func init() {
  envy.Set("GO_ENV", "<environment>")
}

That's might be enough to do it.

@ntakouris
Copy link
Contributor Author

Should it be just Set or MustSet (Try to persist to the host as well)?

@ntakouris ntakouris reopened this Jan 11, 2018
Update bin.go

Update exec.go

Update build.go

Update build.go

change cmd.Env to whole envy.Environ instead

burn GO_ENV into binary

Update build.go

Update options.go

remove forgotten unused commit

asdf
markbates added a commit that referenced this pull request Jan 12, 2018
@markbates
Copy link
Member

@Zarkopafilis I'm closing this PR in favor of #850 because GitHub won't let me push a commit to this PR.

When testing this I found that it still wasn't working. I dug deeper and found that the place that we wanted to set the ENV var was too late. The actions package was already loaded by that point, so the actions.ENV was already set.

In order to get it to load properly, I moved the setting of the ENV var from the main package to the a package which gets loaded before actions. That solved the problem. :)

Thanks for taking point on this.

@markbates markbates closed this Jan 12, 2018
markbates added a commit that referenced this pull request Jan 12, 2018
…e841

re-arranged the implementation of issue #841 to make sure it's loaded before the actions package. fixes #841 and fixes #846
ntakouris pushed a commit to ntakouris/buffalo that referenced this pull request Jan 21, 2018
markbates added a commit that referenced this pull request Jan 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants