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

Pull submodules #1583

Merged
merged 1 commit into from
Jul 16, 2020
Merged

Conversation

mspang
Copy link
Contributor

@mspang mspang commented Jul 13, 2020

Via scripts/helpers/pull_submodules.sh

Updates:
git -C examples/common/m5stack-tft/repo log 42cadf2..42cadf2
git -C third_party/openthread/repo log 5f0e36eeb..b9a7903f1
git -C third_party/ot-br-posix/repo log 77a9156..eefef71
git -C third_party/pigweed/repo log 023f35b..34d9b07

@rwalker-apple
Copy link
Contributor

should we have a blurb about what we're picking up from these updates?

@mspang
Copy link
Contributor Author

mspang commented Jul 13, 2020

should we have a blurb about what we're picking up from these updates?

Do you mean appending something like --pretty=oneline in the description or something manual?

@rwalker-apple
Copy link
Contributor

I was thinking that some rationale would be offered, but perhaps that could be a policy for non-master branches.

@mspang
Copy link
Contributor Author

mspang commented Jul 14, 2020

I was thinking that some rationale would be offered, but perhaps that could be a policy for non-master branches.

Current rationale is

  • m5stack-tft is out of sync between repos.conf & submodules again.
  • We're not testing new changes in the dependencies unless we keep them updated (better to catch problems early).

Copy link
Contributor

@turon turon left a comment

Choose a reason for hiding this comment

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

This should be okay, but we'll need to think about what our policy should be for updating openthread and https://github.com/openthread/ot-br-posix.git such that possilble regressions are avoided. once we have stable Thread support. Perhaps a good simulation framework such as cirque can help here. @gjc13

@rwalker-apple
Copy link
Contributor

I was thinking that some rationale would be offered, but perhaps that could be a policy for non-master branches.

Current rationale is

  • m5stack-tft is out of sync between repos.conf & submodules again.

Is this is addressable with use of a specific hash instead of a branch (as repos.conf is just a symlink)?

  • We're not testing new changes in the dependencies unless we keep them updated (better to catch problems early).

Fair enough for master.

@mspang
Copy link
Contributor Author

mspang commented Jul 14, 2020

I was thinking that some rationale would be offered, but perhaps that could be a policy for non-master branches.

Current rationale is

  • m5stack-tft is out of sync between repos.conf & submodules again.

Is this is addressable with use of a specific hash instead of a branch (as repos.conf is just a symlink)?

It's using specific hashes, however the hash in .gitmodules has fallen out of sync with the one in the git tree. We can certainly sync them back up without pulling everything, though.

  • We're not testing new changes in the dependencies unless we keep them updated (better to catch problems early).

Fair enough for master.

@rwalker-apple
Copy link
Contributor

I was thinking that some rationale would be offered, but perhaps that could be a policy for non-master branches.

Current rationale is

  • m5stack-tft is out of sync between repos.conf & submodules again.

Is this is addressable with use of a specific hash instead of a branch (as repos.conf is just a symlink)?

It's using specific hashes, however the hash in .gitmodules has fallen out of sync with the one in the git tree. We can certainly sync them back up without pulling everything, though.

I think that the repos.conf workflow is unaware of the hashes, only git submodules is. That's what produces the skew. I'm suggesting that the .gitmodules file use an @ format so that the repos.conf workflow matches the git submodules workflow.

@mspang
Copy link
Contributor Author

mspang commented Jul 14, 2020

I was thinking that some rationale would be offered, but perhaps that could be a policy for non-master branches.

Current rationale is

  • m5stack-tft is out of sync between repos.conf & submodules again.

Is this is addressable with use of a specific hash instead of a branch (as repos.conf is just a symlink)?

It's using specific hashes, however the hash in .gitmodules has fallen out of sync with the one in the git tree. We can certainly sync them back up without pulling everything, though.

I think that the repos.conf workflow is unaware of the hashes, only git submodules is. That's what produces the skew. I'm suggesting that the .gitmodules file use an @ format so that the repos.conf workflow matches the git submodules workflow.

Actually we already fixed that. That's what the "commit" lines in the .gitmodules are.

But you either need to use pull_submodules.sh to update both at the same time, or keep them in sync manually.

@andy31415
Copy link
Contributor

@mspang - requires conflict fixing before merge

@mspang mspang force-pushed the for-chip/pull-submodules branch from a6c4ad6 to 30a7110 Compare July 16, 2020 19:45
Via scripts/helpers/pull_submodules.sh

Updates:
    git -C examples/common/m5stack-tft/repo log 42cadf2..42cadf2
    git -C third_party/openthread/repo log 4806fa4d0..c6d5c4aae
    git -C third_party/ot-br-posix/repo log 77a9156..250c6d6
    git -C third_party/pigweed/repo log 023f35b..ce87bc0
@mspang mspang force-pushed the for-chip/pull-submodules branch from 30a7110 to aa83e9d Compare July 16, 2020 19:46
@github-actions
Copy link

Size increase report for "gn_nrf-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@github-actions
Copy link

Size increase report for "gn_linux-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv


@github-actions
Copy link

Size increase report for "nrf-example-build"

File Section File VM
chip-nrf52840-lock-example.out .text 992 992
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-nrf52840-lock-example.out and ./pull_artifact/chip-nrf52840-lock-example.out:

sections,vmsize,filesize
.text,992,992
.strtab,0,752
.symtab,0,256
[Unmapped],0,-992


@github-actions
Copy link

Size increase report for "linux-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-standalone-demo.out and ./pull_artifact/chip-standalone-demo.out:

sections,vmsize,filesize


@github-actions
Copy link

Size increase report for "esp32-example-build"

File Section File VM
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-wifi-echo.elf and ./pull_artifact/chip-wifi-echo.elf:

sections,vmsize,filesize


@andy31415 andy31415 merged commit 9f02853 into project-chip:master Jul 16, 2020
@mspang mspang deleted the for-chip/pull-submodules branch July 16, 2020 21:17
kedars pushed a commit to kedars/connectedhomeip that referenced this pull request Jul 21, 2020
Via scripts/helpers/pull_submodules.sh

Updates:
    git -C examples/common/m5stack-tft/repo log 42cadf2..42cadf2
    git -C third_party/openthread/repo log 4806fa4d0..c6d5c4aae
    git -C third_party/ot-br-posix/repo log 77a9156..250c6d6
    git -C third_party/pigweed/repo log 023f35b..ce87bc0
kedars pushed a commit to kedars/connectedhomeip that referenced this pull request Jul 21, 2020
Via scripts/helpers/pull_submodules.sh

Updates:
    git -C examples/common/m5stack-tft/repo log 42cadf2..42cadf2
    git -C third_party/openthread/repo log 4806fa4d0..c6d5c4aae
    git -C third_party/ot-br-posix/repo log 77a9156..250c6d6
    git -C third_party/pigweed/repo log 023f35b..ce87bc0
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.

6 participants