Skip to content

Update submodules while running build.sh, and make it runs well #11

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

Merged
merged 2 commits into from
Dec 18, 2015

Conversation

TheKK
Copy link
Contributor

@TheKK TheKK commented Dec 18, 2015

No description provided.

@kripken
Copy link
Member

kripken commented Dec 18, 2015

I actually don't know git submodules that well - what do those commands do? I wouldn't want them to pull new changes from those repos, as I need to pull them manually and fix things when they break.

@jcbeyler
Copy link

Agreed, I do that manually to keep the HEAD sane. Doing it at build time is really an invitation to having issues for the user / testers / etc.

@jcbeyler
Copy link

Plus bug reports would become really annoying since each user might have a different configuration depending on what is happening in the submodules compared to what you have or will have by the time you test...

@binji
Copy link
Member

binji commented Dec 18, 2015

None of those commands pull new changes, see https://git-scm.com/docs/git-submodule.

@jcbeyler
Copy link

True I read way too fast the code. The pull command would be adding new updates :)

@TheKK
Copy link
Contributor Author

TheKK commented Dec 18, 2015

git submodule update won't checkout the latest commit for submodules, but stay at the commit when you add it as submodule unless you manually checkout to different commit in submodules then commit changes in binaryen.

Perhapse updating submodule in build.sh is not the best idea. But since files from these submodules are needed while running check.py, I think we should tell others to download them or help them with that.

@kripken
Copy link
Member

kripken commented Dec 18, 2015

I see, thanks. How about creating an update.sh that does this? And perhaps warn from check.py if those directories are not created/up to date?

On the other hand, keeping it in build.sh as currently done in this pull is nice for simplicity, it would just work for most people. I guess I don't feel strongly either way.

@jcbeyler
Copy link

Actually you could put an option to build.sh that does the update or not and then test for that option. At least that way you don' t have to have two different scripts, no?

It' s mostly a build time question. Every time you build you will have to wait for git to finish checking the submodules. So depending on how often you are building, you may or not start caring about this

@TheKK
Copy link
Contributor Author

TheKK commented Dec 18, 2015

As I know binaryen is now seeking for new build system, does that means build.sh will gone in the future? If so, maybe we could just put submodule update instruction to README, otherwise adding an option to build.sh sounds nice.

@kripken
Copy link
Member

kripken commented Dec 18, 2015

Yeah, we need a proper build system anyhow, so build.sh will be going away, I hope ;)

So I'm ok to merge this as-is. Although, the idea of putting the info in the README instead sounds better actually.

@kripken
Copy link
Member

kripken commented Dec 18, 2015

Thinking some more, let's do both, I'll merge this and add it to the readme.

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