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

Add check if submodules need to be updated #4501

Merged
merged 1 commit into from
Oct 7, 2018

Conversation

TheCharlatan
Copy link
Contributor

@TheCharlatan TheCharlatan commented Oct 5, 2018

This is a replacement pr for #3926 . I am not sure this actually does what I inteded. Please suggest better ways to make the check.

Adds CMake check that checks if the commited hash of the submodule is the same as the currently checked out hash in the submodule directory.

@hyc
Copy link
Collaborator

hyc commented Oct 5, 2018

Why bother checking? just run git submodule update and it will either do something or be a no-op.

@TheCharlatan
Copy link
Contributor Author

Do you mean why bother with the pr at all, or why bother using git pull?
Using git pull:

  • In Add submodule init and update to cmake #3926 the check was made with a submodule update, but it was deemed too intrusive
  • It is not possible to dry run a submodule update
    Why bother at all:
  • People forget to run it constantly, which will lead to issues being opened and institutions running unpatched/incorrectly patched software

@moneromooo-monero
Copy link
Collaborator

I see the MANUAL_SUBMODULES flag, which can allow this to be bypassed, and that makes me think of this %@!&*%# firefox which pretty much always enables privacy invading stuff by default, which means that if you don't know in advance what's coming, you run that stuff at least once and curse seven generations of its children before you have a chance to disable the new trap. And that's if you're lucky and realize they added somehting.

Here it's the same: you'll get to run git submodule update at least once before you realize this is now done automatically. That said, it's something needed, not some shitty thing some marketer wanted, so much less bad, but I really wanted to rant about firefox's underhanded ways today, so there you go.

Anyway, looking at the patch, you still hit the network I think. I just looked up where the submodule hashes are kept:

git ls-tree HEAD external/unbound | awk 'print $3'

You can then compare with the top hash, and can either go ahead or error out without hitting the network.

@moneromooo-monero
Copy link
Collaborator

But then the wallet does a DNS lookup. Hmm. What to do...

Adds CMake check that pulls from the different git remotes and checks if
there is any output.
@TheCharlatan
Copy link
Contributor Author

Force pushed a new approach as to @moneromooo-monero 's suggestion. Updated PR description.

Copy link
Contributor

@fluffypony fluffypony left a comment

Choose a reason for hiding this comment

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

Reviewed

@fluffypony fluffypony merged commit fb3593c into monero-project:master Oct 7, 2018
fluffypony added a commit that referenced this pull request Oct 7, 2018
fb3593c Add check if submodules need to be updated (TheCharlatan)
fluffypony added a commit that referenced this pull request Oct 7, 2018
fb3593c Add check if submodules need to be updated (TheCharlatan)
@TheCharlatan TheCharlatan deleted the redoSubmodules branch October 17, 2018 12:00
fotolockr pushed a commit to fotolockr/monero that referenced this pull request May 1, 2019
fb3593c Add check if submodules need to be updated (TheCharlatan)
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.

4 participants