Skip to content

Makefile: fix DESTDIR and PREFIX concatenation#5662

Merged
AkihiroSuda merged 1 commit intocontainerd:mainfrom
thaJeztah:fix_destdir
Jun 25, 2021
Merged

Makefile: fix DESTDIR and PREFIX concatenation#5662
AkihiroSuda merged 1 commit intocontainerd:mainfrom
thaJeztah:fix_destdir

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

Commits 77374e8 (#5577) and b5f530a #5535) changed handling of the DESTDIR and PREFIX variables, and introduced a MANDIR variable.

However, in those commits, the variables are concatenated with a directory separator (/); $DESTDIR/$PREFIX. The $PREFIX variable (and consequently, the MANDIR variable) already should have a leading / (absolute path), so there should be no need to add it. In addition, adding the /, would not allow either an empty path to be passed (well, it would result in // in the path), or for $PREFIX to be used with a relative path (with an empty $PREFIX).

This patch removes the directory separator.

Commits 77374e8 and b5f530a changed handling of the `DESTDIR` and `PREFIX`
variables, and introduced a `MANDIR` variable.

However, in those commits, the variables are concatenated with a directory
separator (`/`); `$DESTDIR/$PREFIX`. The `$PREFIX` variable (and consequently,
the `MANDIR` variable) already should have a leading `/` (absolute path), so
there should be no need to add it. In addition, adding the `/`, would not allow
either an empty path to be passed (well, it would result in `//` in the path),
or for `$PREFIX` to be used with a relative path (with an empty `$PREFIX`).

This patch removes the directory separator.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda AkihiroSuda merged commit e72a56a into containerd:main Jun 25, 2021
@thaJeztah thaJeztah deleted the fix_destdir branch June 25, 2021 15:37
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.

3 participants