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

Cleanup make dist #8024

Merged
merged 1 commit into from
Aug 20, 2014
Merged

Cleanup make dist #8024

merged 1 commit into from
Aug 20, 2014

Conversation

staticfloat
Copy link
Sponsor Member

  • No longer install anything related to libuv with make dist
  • Use custom install imposter; copy symlinks correctly to drastically reduce make dist size.

Tested on OSX and Linux (Ubuntu)

@staticfloat staticfloat mentioned this pull request Aug 16, 2014
4 tasks
DESTFILE=$(echo $DESTFILE | sed -e 's/[‘]\(.*\)[’]/\1/' )

# Do the chmod dance, but only if this is not a symlink (on linux)
if [[ "$(uname -s)" == "Linux" && ! -h $DESTFILE ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

would this be needed on freebsd too?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Looking at this again, I completely flummoxed this check. I have a much more resilient method in mind.

@tkelman
Copy link
Contributor

tkelman commented Aug 16, 2014

Oh and for BSD's this shouldn't rely on bash being installed. Posix sh only.

@staticfloat
Copy link
Sponsor Member Author

Good reminder. I always default to bash for some reason. :P

@tkelman
Copy link
Contributor

tkelman commented Aug 16, 2014

Travis says line 10 is a bashism

@staticfloat
Copy link
Sponsor Member Author

It wasn't the only one! :P I think I've got everything working now.

@JeffBezanson
Copy link
Sponsor Member

Sweet!

@tkelman
Copy link
Contributor

tkelman commented Aug 17, 2014

Wait, do we want to install versioned symlinks at all? Maybe install is doing what we want it to but we're just calling it on things we shouldn't be?

@staticfloat
Copy link
Sponsor Member Author

It is best practice to install the versioned symlinks. I see no reason not to, since it's completely valid to try and dlopen libopenblas.so.0.2.10, and without these files here, that will fail.

@tkelman
Copy link
Contributor

tkelman commented Aug 17, 2014

As long as the symlinks are all relative-paths, get skipped in the Windows build (I can't find any on master so I think we're okay), and get properly preserved by archives, I guess it's fine then.

@staticfloat
Copy link
Sponsor Member Author

Yes, they are definitely all relative. I think they don't get installed on Windows because the individual build processes for OpenBLAS, openlibm, etc... just don't create them on Windows. (At least, that's my guess without looking into it). Symlinks are preserved in .tar files.

@tkelman
Copy link
Contributor

tkelman commented Aug 17, 2014

Oddly libgmp does make a symlink at usr/lib/libgmp.lib (funny that a GNU library would get their autotools so wrong there both in symlinking and the static library extension for mingw), but we don't install it or put that file in the binaries. MSYS2's tar has trouble with symlinks (see libgit2/libgit2#2526 for example), but there's not too much reason I can think of to need to work with the OSX/Linux binaries from within MSYS2.

@JeffBezanson
Copy link
Sponsor Member

So we will not have this for 0.3?

@staticfloat
Copy link
Sponsor Member Author

Ack, I thought this was merged days ago. We should probably have this to reduce distribution size, huh.

@staticfloat
Copy link
Sponsor Member Author

Let me squash this, and then I'll merge and do some rebase magic.

* No longer install anything related to libuv with `make dist`

* Use custom `install` imposter; copy symlinks correctly to drastically reduce `make dist` size.
staticfloat added a commit that referenced this pull request Aug 20, 2014
@staticfloat staticfloat merged commit 73bb870 into master Aug 20, 2014
@ivarne
Copy link
Sponsor Member

ivarne commented Aug 20, 2014

Isn't this what we have milestones for?

@staticfloat
Copy link
Sponsor Member Author

Yes, I should have put this on a milestone.

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.

None yet

5 participants