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

implement DESTDIR support for distros #3053

Merged
merged 1 commit into from
Aug 22, 2023
Merged

Conversation

juhp
Copy link
Contributor

@juhp juhp commented Aug 20, 2023

Description

Prefix ${PREFIX} with ${DESTDIR} allowing installation into a staging directory for packaging Idris2.

Also take DESTDIR into account with idris2 --install etc.

Should this change go in the CHANGELOG?

  • If this is a fix, user-facing change, a compiler change, or a new paper
    implementation, I have updated CHANGELOG.md (and potentially also
    CONTRIBUTORS.md).

I tested this a little on Fedora Linux.

When DESTDIR is not set, installation should behave exactly as now.

both 'make install' and 'idris2 --install*' should respect DESTDIR now
@juhp
Copy link
Contributor Author

juhp commented Aug 21, 2023

I would appreciate if someone could just have a glance at my simple idris code

Copy link
Member

@gallais gallais left a comment

Choose a reason for hiding this comment

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

Should PREFIX be computed from DESTDIR rather than prepending DESTDIR everywhere (and risking to forget it in places)?

@mattpolzin
Copy link
Collaborator

Should PREFIX be computed from DESTDIR

That might be more confusing than not since DESTDIR is documented as both "only applied in install steps" and "never showing up in built files" (like IdrisPaths.idr). So, for example, we would need to be careful to track where PREFIX gets used very carefully and also not redefine it prior to setting IDRIS2_PREFIX based on it.

OTOH, I think I'd be ok with either the current approach or a new variable like INSTALL_PREFIX. Would still perhaps benefit from explicit comment that such a variable is only to be used to offset installation and not to impact any build operations.

@juhp
Copy link
Contributor Author

juhp commented Aug 22, 2023

Right, this patch tries to do the minimal simplest thing, so that it should be obvious it does no harm.
DESTDIR support has been missing for a long time.
Perhaps further down the road it could be integrated more deeply.

@gallais gallais merged commit 48dbc32 into idris-lang:main Aug 22, 2023
20 checks passed
@gallais gallais added the Installation Issue Problem compiling or running Idris label Aug 22, 2023
@juhp juhp deleted the DESTDIR branch August 22, 2023 15:45
@juhp
Copy link
Contributor Author

juhp commented Aug 22, 2023

Thank you @gallais for reviewing :yay:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Installation Issue Problem compiling or running Idris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants