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

Get habitat build working with changes from #1654 #1658

Closed
wants to merge 1 commit into from
Closed

Conversation

jaym
Copy link
Contributor

@jaym jaym commented Apr 3, 2019

The pwd for building changed with #1654. Hopefully
this makes it not matter

Signed-off-by: Jay Mundrawala [email protected]

@jaym jaym requested a review from a team April 3, 2019 20:31
The pwd for building changed with #1654. Hopefully
this makes it not matter

Signed-off-by: Jay Mundrawala <[email protected]>
Copy link
Contributor

@markan markan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful to have a little more explanation of this change, at least for the benefit of not accidentally breaking things later.

@@ -23,11 +23,19 @@ pkg_binds_optional=(
[chef-server-ctl]="secrets"
)

# set_pwd gives a consistent current working directory when building
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit I'm a little unclear about the impact of this; why is it necessary? How does it affect local package builds (non-expeditor)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think tom is going to do something slightly different. Basically, with the new build, it's doing the equivalent of build src/bifrost/habitat instead of what we were doing before build src/bifrost. Hab sets different working directories for these, and our plans relied on the working directory to work correctly, so they don't work with the new build changes

Copy link
Contributor

@markan markan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this is fixing some breakage, so we should merge; I'll bug people out of band to get up to speed on the details.

@jaym
Copy link
Contributor Author

jaym commented Apr 4, 2019

@tduffield is doing stuff to fix this

@jaym jaym closed this Apr 4, 2019
@markan markan deleted the jdm/fix-build branch April 10, 2019 01:36
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.

2 participants