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

Add make install #10878

Merged

Conversation

straight-shoota
Copy link
Member

Adds Makefile recipe to install the compiler.

I separated install_docs as a separate recipe because the (uncompressed) API docs are quite large in file size and not all install targets might want to include them (the generic Linux tarball for example doesn't). So in order to install the compiler with API docs, you need to run make install install_docs.

Resolves #10702

Makefile Outdated
$(INSTALL) -d -m 0755 $(DATADIR)
cp -av src "$(DATADIR)/src"
rm -rf "$(DATADIR)/$(LLVM_EXT_OBJ)" # Don't install llvm_ext.o
cp -av samples "$(DATADIR)/examples"
Copy link
Member

Choose a reason for hiding this comment

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

Should this be part of the docs target perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that might be better. I used the current distribution-scripts as role model and the samples are part of the program tarball. But I agree it would make more sense to place them in the docs package.
I also don't really understand why it's renamed from samples to examples. Should be one or the other.

@@ -46,6 +46,14 @@ DEPS = $(LLVM_EXT_OBJ)
CXXFLAGS += $(if $(debug),-g -O0)
CRYSTAL_VERSION ?= $(shell cat src/VERSION)

DESTDIR ?=
PREFIX ?= /usr/local
Copy link
Member

Choose a reason for hiding this comment

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

What's the rationale for having DESTDIR in addition to PREFIX?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're already using the same setup in shards. And it seems to be commonly used (https://stackoverflow.com/questions/11307465/destdir-and-prefix-of-make).
The original purpose is probably that the path PREFIX could be baked into the binary, so it's set at ./configure. And DESTDIR is only used for the install recipe directly. We wouldn't really need that with #10877 and can get the same functionality with PREFIX only (which can include the path to DESTDIR). But I think it still makes sense to have both.

$(INSTALL) -D -m 0755 "$(O)/crystal" "$(BINDIR)/crystal"

$(INSTALL) -d -m 0755 $(DATADIR)
cp -av src "$(DATADIR)/src"
Copy link
Member

@jhass jhass Jul 5, 2021

Choose a reason for hiding this comment

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

At least Ruby and Python put their standard library into /usr/lib. Why is /usr/share better? What's the advantage of including a src subdirectory?

Copy link
Member Author

Choose a reason for hiding this comment

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

Again, this structure is directly copied from the tarball generation in distribution-scripts.

I don't know the specific reasons for lib vs. share. But in the Filesystem Hierarchy Standard, ˋ/usr/libˋ is for (binary) libraries and ˋ/usr/shareˋ for architecture-independent data. Ths stdlib source code is architecture-independent, so I suppose it makes sense to place it there. Not sure why Ruby and Python put it in ˋlibˋ.

The reason for the ˋsrcˋ folder is because there's other data that is to be placed beside it, for example ˋdocsˋ and ˋsamplesˋ.

Copy link
Member

@jhass jhass Jul 5, 2021

Choose a reason for hiding this comment

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

/usr/lib includes object files and libraries. On some systems, it may also include internal binaries that are not intended to be executed directly by users or shell scripts.

Applications may use a single subdirectory under /usr/lib. If an application uses a subdirectory, all architecture-dependent data exclusively used by the application must be placed within that subdirectory.

https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s06.html

Where does this say only architecture dependent or binary?

Just looking randomly through my /usr/lib I find "text" files by:

  • Go

  • NodeJS

  • Ruby

  • Python

  • TCL

  • udev

  • systemd

  • resolveconf

Furthermore precompiled libraries by

  • Erlang
  • OCaml

/usr/share seems preferred by Perl and Vala only from a quick look.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, the exact semantics of the specification may be subject to interpretation. They're not completely clear.

But I don't think this really matters much. /usr/share/crystal is not wrong, so there's no reason against it. We're using this path in all distribution packages (at least for linux, not sure about mac). I don't see much reason to change it.

Copy link
Member

@jhass jhass Jul 5, 2021

Choose a reason for hiding this comment

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

There would be no need for that weird src subdirectory? :P

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not? And why is it weird?

At /usr/lib/crystal there's already a subfolder lib which contains the bundled libraries (currently only libgc.a). If we would put the source files there too, they should obviously go into another subdirectory.

@straight-shoota
Copy link
Member Author

With #11030 being merged, make install works really great. The compiler automatically picks up the stdlib location.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

🚀

@straight-shoota straight-shoota added this to the 1.2.0 milestone Sep 15, 2021
@straight-shoota straight-shoota merged commit 3d2fae5 into crystal-lang:master Sep 17, 2021
@straight-shoota straight-shoota deleted the feature/make-install branch September 17, 2021 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add make install command when building from source
3 participants