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.

Sign up for GitHub

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

Jump to bottom

[build] add avahi daemon and library #3263

Merged
merged 5 commits into from
Oct 21, 2020
Merged

[build] add avahi daemon and library #3263

merged 5 commits into from
Oct 21, 2020

Conversation

gjc13
Copy link
Contributor

@gjc13 gjc13 commented Oct 15, 2020

Problem

#3241 requires avahi as build dependency.

Summary of Changes

Adds avahi daemon and lib to the build docker.

bukepo
bukepo approved these changes Oct 15, 2020
View reviewed changes
Copy link
Contributor

@bukepo bukepo left a comment

Choose a reason for hiding this comment

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

LGTM:+1:

@andy31415
Copy link
Contributor

@gjc13 - you need to update the version file as well since the image changes.

@gjc13
Copy link
Contributor Author

gjc13 commented Oct 16, 2020

@gjc13 - you need to update the version file as well since the image changes.

Updated.

@mspang mspang requested a review from rwalker-apple October 19, 2020 23:43
rwalker-apple
rwalker-apple reviewed Oct 20, 2020
View reviewed changes
integrations/docker/run.sh Outdated
@@ -35,7 +35,7 @@ ORG=${DOCKER_RUN_ORG:-connectedhomeip}
IMAGE=${DOCKER_RUN_IMAGE:-$(basename "$here")}

# version
VERSION=${DOCKER_RUN_VERSION:-$(cat "$here/version")} ||
VERSION=${DOCKER_RUN_VERSION:-latest} ||
Copy link
Contributor

Choose a reason for hiding this comment

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

we haven't been keeping latest up to date...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using $(cat "$here/version") with cause the cirque build to fail. Is it okay to ignore it?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably just use a specific version like 0.4.11, like GitHub actions does. That avoids the circular dependency when updating the dockerfile.

Copy link
Contributor

Choose a reason for hiding this comment

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

  • can't ignore cirque builds
  • can't use a specific version (because version is shared, via symlinks)

For this PR to land, all of the images need to be pushed first.

This should probably be looked at again. When I wrote all this, it was expedient to bump all versions all the time because chip-build was the image that changed most often and all the children needed the latest chip-build.

Today, we can version chip-build independently and let children update as necessary.

Changes needed:

  • images/*/version files to be un-symlinked
  • Dockerfile FROM directives to use specific versions
  • integrations/docker/images/build-all.sh to be reworked with the above in mind

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've unlinked the version file in build-cirque and fix it to v0.4.11 as a temporary fix.

@wgtdkp wgtdkp mentioned this pull request Oct 21, 2020
Merged
@mspang mspang requested a review from rwalker-apple October 21, 2020 02:53
@mspang
Copy link
Contributor

mspang commented Oct 21, 2020 •
edited
Loading

Reminder to use the "re-request review" button.

@rwalker-apple rwalker-apple mentioned this pull request Oct 21, 2020
Closed
@rwalker-apple rwalker-apple merged commit 7331637 into project-chip:master Oct 21, 2020
jrhees-cae added a commit to jrhees-cae/connectedhomeip that referenced this pull request Aug 3, 2023
@jrhees-cae
TC-DRLK-2.2/2.3/2.12:
5fca778 
- Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE
- Reorder steps 7-9 to ensure subsequent test of WrongCodeEntryLimit doesn't already have existing failed entry attempts
- Fix incorrect PICS (use .Write PICS) for writing WrongCodeEntryLimit and UserCodeTemporaryDisableTime

Fixes project-chip#3263
Fixes project-chip#3265
Fixes project-chip#3266
@jrhees-cae jrhees-cae mentioned this pull request Aug 3, 2023
Closed
jrhees-cae added a commit to jrhees-cae/connectedhomeip that referenced this pull request Aug 4, 2023
@jrhees-cae
TC-DRLK-2.2/2.3/2.12: Modify flow to allow for RequirePINforRemoteOpe…
c6a52b1 
…ration to be TRUE or FALSE

- Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE
- Reorder steps 7-9 to ensure subsequent test of WrongCodeEntryLimit doesn't already have existing failed entry attempts
- Fix incorrect PICS (use .Write PICS) for writing WrongCodeEntryLimit and UserCodeTemporaryDisableTime

Fixes project-chip#3263
Fixes project-chip#3265
Fixes project-chip#3266
mergify bot pushed a commit that referenced this pull request Aug 10, 2023
@jrhees-cae @restyled-commits
TC-DRLK-2.2/2.3/2.12: Modify flow to allow for RequirePINforRemoteOpe…
1b6ea10 
…ration to be TRUE or FALSE (#28523)

* TC-DRLK-2.2/2.3/2.12: Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE

- Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE
- Reorder steps 7-9 to ensure subsequent test of WrongCodeEntryLimit doesn't already have existing failed entry attempts
- Fix incorrect PICS (use .Write PICS) for writing WrongCodeEntryLimit and UserCodeTemporaryDisableTime

Fixes #3263
Fixes #3265
Fixes #3266

* Restyled by autopep8

* Exclude .yaml tests from CI due to use of EqualityTests pseudo-cluster

* Add zap-regen'd files

* Exclude .yaml tests from CI linux due to use of EqualityTests pseudo-cluster

---------

Co-authored-by: Restyled.io <[email protected]>
abpoth pushed a commit to abpoth/connectedhomeip that referenced this pull request Aug 15, 2023
@jrhees-cae @restyled-commits
TC-DRLK-2.2/2.3/2.12: Modify flow to allow for RequirePINforRemoteOpe…
a66c77c 
…ration to be TRUE or FALSE (project-chip#28523)

* TC-DRLK-2.2/2.3/2.12: Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE

- Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE
- Reorder steps 7-9 to ensure subsequent test of WrongCodeEntryLimit doesn't already have existing failed entry attempts
- Fix incorrect PICS (use .Write PICS) for writing WrongCodeEntryLimit and UserCodeTemporaryDisableTime

Fixes project-chip#3263
Fixes project-chip#3265
Fixes project-chip#3266

* Restyled by autopep8

* Exclude .yaml tests from CI due to use of EqualityTests pseudo-cluster

* Add zap-regen'd files

* Exclude .yaml tests from CI linux due to use of EqualityTests pseudo-cluster

---------

Co-authored-by: Restyled.io <[email protected]>
s07641069 pushed a commit to s07641069/connectedhomeip that referenced this pull request Aug 22, 2023
@jrhees-cae @restyled-commits @s07641069
TC-DRLK-2.2/2.3/2.12: Modify flow to allow for RequirePINforRemoteOpe…
1d05925 
…ration to be TRUE or FALSE (project-chip#28523)

* TC-DRLK-2.2/2.3/2.12: Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE

- Modify flow to allow for RequirePINforRemoteOperation to be TRUE or FALSE
- Reorder steps 7-9 to ensure subsequent test of WrongCodeEntryLimit doesn't already have existing failed entry attempts
- Fix incorrect PICS (use .Write PICS) for writing WrongCodeEntryLimit and UserCodeTemporaryDisableTime

Fixes project-chip#3263
Fixes project-chip#3265
Fixes project-chip#3266

* Restyled by autopep8

* Exclude .yaml tests from CI due to use of EqualityTests pseudo-cluster

* Add zap-regen'd files

* Exclude .yaml tests from CI linux due to use of EqualityTests pseudo-cluster

---------

Co-authored-by: Restyled.io <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mspang mspang mspang approved these changes

@andy31415 andy31415 andy31415 approved these changes

@bukepo bukepo bukepo approved these changes

@saurabhst saurabhst saurabhst approved these changes

@rwalker-apple rwalker-apple rwalker-apple approved these changes

@BroderickCarlin BroderickCarlin Awaiting requested review from BroderickCarlin

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@hawk248 hawk248 Awaiting requested review from hawk248

@jelderton jelderton Awaiting requested review from jelderton

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

Assignees
No one assigned
Labels
integrations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@gjc13 @andy31415 @mspang @bukepo @saurabhst @rwalker-apple