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

openlibm is no longer managed as a git submodule #10177

Closed
wants to merge 1 commit into from

Conversation

ViralBShah
Copy link
Member

We now use openlibm releases
Add openlibm checksums
Update gitignore

Verified

This commit was signed with the committer’s verified signature.
renovate-bot Mend Renovate
We now use openlibm releases
Add openlibm checksums
Update gitignore
@ViralBShah
Copy link
Member Author

It seems that on Windows, julia/src/support/*longjmp* files depend on the presence of bsd_asm.h in deps/openlibm/. Perhaps we should just copy the relevant headers into julia/src.

Cc: @vtjnash

@ViralBShah
Copy link
Member Author

    CC src/support/_setjmp.win32.o
_setjmp.win32.S:42:46: fatal error: ../../deps/openlibm/i387/bsd_asm.h: No such file or directory
 #include "../../deps/openlibm/i387/bsd_asm.h"

@vtjnash
Copy link
Member

vtjnash commented Feb 12, 2015

What's the benefit here?

@ViralBShah
Copy link
Member Author

It just seems strange for code in src to depend on code in openlibm. Why not have the julia source have its own copy of the headers it needs? One could easily have moved files around in openlibm without knowing this and easily broken the build.

The main reason to do this is that with openlibm having releases, it is simpler to have julia depend on released versions.

@StefanKarpinski
Copy link
Member

I have to say I'm not clear on why we'd want to do this either.

@ViralBShah
Copy link
Member Author

Openlibm has seen quite a few changes, and bundling a specific commit just does not seem like a good idea. One could always use the commit from a released version - so yes, this is not strictly necessary.

In any case, even if people don't like this, I still find the code dependency weird.

@nalimilan
Copy link
Member

That dependency on the sources being in ../deps is pretty ugly indeed. I think I removed these on Linux to be able to build RPM packages, but I've probably missed Windows.

@tkelman
Copy link
Contributor

tkelman commented Feb 13, 2015

I don't like and have complained about that hard-coded relative path dependency to private openlibm headers before, and warned that the changes to openlibm could cause trouble here. But I don't like copying the headers into Julia's repo much either. Might be nice to clean up those assembly files so they can be self-contained and stop relying on openlibm headers, but there are better things to worry about. Keeping it as a submodule isn't the end of the world.

What would be more important long-term is working on our other big submodule, coming up with a way to use upstream libuv instead of our own fork. We haven't rebased in a long time, and there were some discussions outlining a path towards upstreaming the missing functionality, just no one has gone and done the work yet.

@vtjnash
Copy link
Member

vtjnash commented Feb 13, 2015

since we can't build against anything but openlibm on windows, it's a pretty minor nit. although, i agree that it does break the usual encapsulation principles.

@tkelman
Copy link
Contributor

tkelman commented Feb 13, 2015

Submodules are pretty annoying and it would be nice to get rid of them, but the actual benefit there doesn't come until we can get rid of all of them.

@ViralBShah
Copy link
Member Author

I don't think we are too far from getting rid of all submodules. This PR does so for openlibm. openspecfun and libmojibake should be easy too. The real issue is libuv - but there given that we still maintain a fork, perhaps we can just have libuv-julia, just like Rmath-julia.

@vtjnash
Copy link
Member

vtjnash commented Feb 14, 2015

I didn't realize that was still a goal. I still find it very helpful that libuv and libopenlibm are git submodules (although I haven't needed to edit them as much recently). It would be OK with me to make more dependencies into submodules, since it would make them easier to manage and modify.

@ViralBShah
Copy link
Member Author

It certainly is easier to manage the dependencies as submodules. The downside is that it lets one get away with less discipline, and makes it difficult for packaging and distribution - especially in the case of distros, where the dependencies are bundled independently. To someone who follows development regularly as folks here, either way is ok, but for others who only jump in during releases, it is nice to have the separation and dependency on released versions of external libraries, rather than specific commits.

Now, if we find bugs in these libraries, we have to do a bit more hard work to ensure that other things did not break and go through a release process (currently, a tag and writeup in most cases). The upstream work becomes a bit more tedious, but I feel it also forces a little more discipline - especially now that there are other people using openlibm outside of julia too. In Julia, it just requires changing deps/Versions.make, which is as easy as managing with git.

@vtjnash
Copy link
Member

vtjnash commented Feb 14, 2015

I'm not convinced by an argument that making more work for ourselves is actually a positive benefit.

the part about making them not managed as submodules is that it makes regression testing the effect of upstream changes, making modifications, and then pushing those upstream is much more tedious. i end up just making lots of copies of the libraries and git managing some random subset of them – it's not pretty, but i have lost almost all of the benefits of these libraries being in a versioned repository.

@ViralBShah
Copy link
Member Author

It may seem like more work during development, but depending on specific commits of upstream libraries means that distros end up shipping buggy versions, which comes back to us in the form of bug reports.

I personally don't like git submodules, but that is a matter of taste. I don't have a strong opinion on the matter. My main point is that it would be nice to work with released versions of libraries when we ship a julia release, and it would be nice for the julia sources not to depend on openlibm the way they do right now.

@ViralBShah ViralBShah added the building Build system, or building Julia or its dependencies label Feb 14, 2015
@ivarne
Copy link
Member

ivarne commented Feb 14, 2015

The pain with submodules is that whenever you decide to bump the sha of one of them, we'll find a few pull requests (or direct commits to master) that reverts the change, because someone didn't run git submodule update before git commit --a.

git add --p does solve the issue (or at least highlight it), but isn't suggested often enough in git tutorials, so many contributors won't use it. I'm really sad when new contributors get a feeling they screwed up, rather than the great feeling of being useful because we use a poorly designed git feature.

Now that we have sha1/md5 verification of downloaded dependencies, the verification value of submodules is gone, so the only thing left is a simplified workflow when developing patches for dependencies. I haven't touched any of the submoduled dependencies, so I'm not a good arbitrator for that decision.

@tkelman
Copy link
Contributor

tkelman commented Feb 14, 2015

The real issue is libuv

Agreed.

but there given that we still maintain a fork, perhaps we can just have libuv-julia

Disagreed. Please no. We have an outstanding project to get our changes incorporated upstream and we won't need a fork any more. It's not a super easy project (GSoC material maybe?), someone just needs to do it. I also wouldn't really call it "maintain"ing a fork when we haven't touched it in months.

I'm really sad when new contributors get a feeling they screwed up, rather than the great feeling of being useful because we use a poorly designed git feature.

Exactly. This is what makes submodules messy - dealing with them is completely non-obvious to newcomers.

I'm in favor of trying to get away from submodules long term, but only when things are ready to do so without it being an annoying transition. Switching Rmath worked. That's also not really a base dependency so it makes sense to decouple it. Openspecfun and/or mojibake would be easy (edit: aren't we planning on upstreaming all the mojibake patches anyway and going back to using utf8proc when it's good enough for our purposes?). Libuv I'd rather not move away from a submodule until we're ready to use an unpatched upstream release.

@tkelman
Copy link
Contributor

tkelman commented Apr 20, 2015

closing this, was done by #10743

@tkelman tkelman closed this Apr 20, 2015
@tkelman tkelman deleted the vs/openlibm branch April 20, 2015 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants