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

[RELEASE] Unbound: bump to 1.7.0rc3 + remove in-tree source, use git submodule #2133

Merged
merged 2 commits into from
Mar 18, 2018

Conversation

anonimal
Copy link
Contributor

As approved by @moneromooo-monero on IRC, still open for consideration: here is the concept of what we should consider doing for all our external dependencies.

To consider:

  • I wish to immediately move https://github.com/anonimal/unbound into monero-project, thus stripping any of my write access to the repo
  • I've kept things simple regarding svn to git: no trunk/branch/tag conversions, no prettying of git log. This has proven to make things easier when we git svn rebase; especially because we are working from a custom branch and we will need to rebase for every unbound release.

Referencing #2089 (comment)

Copy link
Collaborator

@moneromooo-monero moneromooo-monero left a comment

Choose a reason for hiding this comment

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

Approved in principle, BUT github does not seem to show the patch setting the commit hash for the submodule, it shows some dodgy looking preparsed link, so it's hard to see what the patch actully is for that (pretty important) part. I think it should be merged only after the URL target switches to monero-project though, even with a set hash.

@anonimal
Copy link
Contributor Author

github does not seem to show the patch setting the commit hash for the submodule, it shows some dodgy looking preparsed link

Yes, it's annoying, but it appears to be accurate. I'm guessing the only other way to verify with confidence (until github fixes, if ever) will be to fetch branch monero from my fork to confirm the commit. I'm thinking people will be doing this anyway, so that's good.

I think it should be merged only after the URL target switches to monero-project

As soon as @fluffypony et al creates monero-project/unbound, I'll move my repo over a.s.a.p.

@fluffypony
Copy link
Contributor

@anonimal
Copy link
Contributor Author

anonimal commented Aug 7, 2017

@fluffypony rebased. I don't have push access to monero-project/unbound (at least temporarily to get my repo moved) so the code will have to get there somehow before this PR is merged.

@fluffypony
Copy link
Contributor

@anonimal can't you PR to that? or must I initialise with a readme for that to work?

@anonimal
Copy link
Contributor Author

can't you PR to that?

Nope. I tried. AFAICT the repo needs to be pushed directly. Ideally (in fact, we probably shouldn't) you don't initialize a README or any file because master will be the pure svn-to-git of unbound and having our own commit will make things a pain to deal with IMHO.

@anonimal
Copy link
Contributor Author

Successfully pushed to https://github.com/monero-project/unbound.

Copy link
Contributor

@danrmiller danrmiller left a comment

Choose a reason for hiding this comment

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

Works for me on linux, windows, freebsd, and mac, and the changes appear to do what the title claims.

@hyc
Copy link
Collaborator

hyc commented Oct 11, 2017

We'll definitely need a pristine branch for syncing from upstream, but also a monero-specific branch for any point patches we need to apply.

@danrmiller
Copy link
Contributor

This submodule already uses the "monero" branch from https://github.com/monero-project/unbound/tree/monero.

We should verify that everything we need is in there and PR what's missing. It looks like many things, for example 45e9838, are already included upstream.

For the "clean" branch, I'll check out upstream with subversion and verify, but it looks like we are tracking trunk. Should we pick a tag instead like either the latest 1.6.7 or the 1.6.3 we were previously using? Or do the differences between svn and git make that not practical?

@moneromooo-monero
Copy link
Collaborator

Is there anything I can help with here ?

@anonimal
Copy link
Contributor Author

Oops, forgot that I opened this PR. When someone gives me the go-ahead, I'll rebase. I'm not sure if @danrmiller had a chance to verify.

Or do the differences between svn and git make that not practical?

AFAICT, and IIRC, not practical (but maybe someone else has input on that).

@anonimal anonimal mentioned this pull request Mar 16, 2018
@anonimal
Copy link
Contributor Author

Ping @danrmiller

@danrmiller
Copy link
Contributor

@anonimal What would you like me to do? I don't have anything to add since last time.

@anonimal anonimal changed the title Unbound: remove in-tree source, use git submodule [RELEASE] Unbound: bump to 1.7.0rc3 + remove in-tree source, use git submodule Mar 16, 2018
@anonimal
Copy link
Contributor Author

@danrmiller ok, thanks for clarifying.

I've opened monero-project/unbound#1 and monero-project/unbound#2. After they are merged, this PR will resolve #3413 (@fluffypony I can't rebase this PR until those are merged).

@anonimal
Copy link
Contributor Author

Note: this PR was marked for release because the in-tree unbound is outdated at 1.6.3. The current 1.7.0rc3 is the latest stable version and is the tag used in their March 15th, release.

We'll instead use a git submodule to pull from our unbound repo.
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 efe70a1 into monero-project:master Mar 18, 2018
fluffypony added a commit that referenced this pull request Mar 18, 2018
efe70a1 Unbound: add git submodule for unbound (anonimal)
84c5a9b Unbound: remove unbound from in-tree source (anonimal)
@anonimal anonimal deleted the unbound branch March 18, 2018 16:52
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.

5 participants