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

Allow arc patch to update submodules #2

Merged
merged 2 commits into from Jul 18, 2015

Conversation

Projects
None yet
5 participants
@adamse
Contributor

adamse commented Jul 3, 2015

By not init'ing and update'ing submodules arc patch does the work and
thus updates to submodules, including changes to submodule urls, will
work smoothely.


Tested by applying https://phabricator.haskell.org/D1033 in two different ways: (since I have no local phab install)

As the run.sh script currently does

git clone GHC
git submodule init
git submodule update
arch patch D1033

Which fails with

...
Applied patch .gitmodules cleanly.
fatal: reference is not a tree: b18d17fc2bbafc6e8576764df8cf299b26fb1835
Unable to checkout 'b18d17fc2bbafc6e8576764df8cf299b26fb1835' in submodule path 'utils/haddock'

And the updated way:

git clone GHC
arc patch D1033

Here arc patch applies the patch, and then init's and update's the submodules, which makes everything work out OK.

Allow arc patch to update submodules
By not init'ing and update'ing submodules arc patch does the work and
thus updates to submodules, including changes to submodule urls, will
work smoothely.
@rwbarton

This comment has been minimized.

rwbarton commented on 9781bb6 Jul 3, 2015

Channeling SPJ, this certainly deserves a comment about why not setting up the submodules here is important, as otherwise it can look like an inadvertent omission.

@thomie

This comment has been minimized.

Contributor

thomie commented Jul 6, 2015

This is great. Could you update one of the following pages:

Maybe with a link from the other page. Explain how to submit a patch to Phabricator that contains submodule changes, like you did in https://phabricator.haskell.org/D1033. It's a nice trick, even though the patch can't be landed as is, it can at least be validated.

@adamse

This comment has been minimized.

Contributor

adamse commented Jul 6, 2015

@thomie I will make sure to update the pages, once I'm sure that it actually works :)

@bgamari

This comment has been minimized.

Contributor

bgamari commented Jul 15, 2015

@adamse, Do let us know when you are comfortable with this. It would be great to have this fixed.

@adamse

This comment has been minimized.

Contributor

adamse commented Jul 15, 2015

@bgamari As long as it actually works with Harbormaster I am comfortable with this!

@thoughtpolice

This comment has been minimized.

Contributor

thoughtpolice commented Jul 18, 2015

LGTM.

thoughtpolice added a commit that referenced this pull request Jul 18, 2015

@thoughtpolice thoughtpolice merged commit 44ffbdd into haskell-infra:master Jul 18, 2015

@thomie

This comment has been minimized.

Contributor

thomie commented Jul 21, 2015

@adamse: if doesn't seem to have worked. I restarted https://phabricator.haskell.org/D1033, but it still fails with haddock errors.

@adamse

This comment has been minimized.

Contributor

adamse commented Jul 21, 2015

@thomie I changed the submodule back to the original after some commits were merged into a WIP branch, but then I realised that more changes were needed, without changing the submodule again.

@bgamari

This comment has been minimized.

Contributor

bgamari commented Jul 21, 2015

It seems that the repo was never updated on the build machine. Trying D1033 again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment