Page MenuHomeFreeBSD

Mk/bsd.port.mk: stop ports from finding the ports tree's git repository
Needs ReviewPublic

Authored by fuz on Sat, Nov 30, 1:39 PM.
Tags
None
Referenced Files
F104077765: D47850.id.diff
Tue, Dec 3, 6:08 AM
F104077740: D47850.id147323.diff
Tue, Dec 3, 6:08 AM
F104077738: D47850.id147183.diff
Tue, Dec 3, 6:08 AM
F104076693: D47850.diff
Tue, Dec 3, 5:48 AM

Details

Reviewers
None
Group Reviewers
portmgr
Summary

When building from source, the ports tree is by default a parent
directory of WRKDIR. When ports try to use git to find the git commit
they are built from, they'll some times find the port's git repository
and produce bogus metadata. Set GIT_CEILING_DIRS in WRK_ENV to ensure
that git does not escape WRKDIR trying to look for a git repository
where we have extracted the distfile.

This doesn't affect Poudriere builds as for these, WRKDIR is by default
not a subdirectory of the ports tree, and neither is git installed in
builder jails unless explicitly needed. As a result, bugs of this kind
tend to go unnoticed by maintainers, but do affect source builders.

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 60902
Build 57786: arc lint + arc unit

Event Timeline

fuz requested review of this revision.Sat, Nov 30, 1:39 PM
fuz created this revision.

This looks like a good idea, but could you please add the new environment variable to where WRK_ENV is first appended to, about 100 lines before.

  • Mk/bsd.port.mk: move GIT_CEILING_DIRECTORIES to initial WRK_ENV block
Mk/bsd.port.mk
1623

Why :H here ? ${WRKDIR} should be enough.

fuz marked an inline comment as done.Mon, Dec 2, 7:19 AM
fuz added inline comments.
Mk/bsd.port.mk
1623

GIT_CEILING_DIRECTORIES is a list of directories git will not change into when looking for git repositories. It is conceivable that a port would use WRKDIR for a git repository (even though that sounds a bit weird), so the first directory that is forbidden is the parent directory of WKRDIR.

And when that is changed, please ask for an exp-run to see if things break.

Mk/bsd.port.mk
1623

Yeah, no, ports are always extracted in a subdirectory of WRKDIR because other things get stored there.
If there are ports that break, they are probably doing something stupid and should be fixed.
Please change it to ${WRKDIR}.

Mk/bsd.port.mk
1623

Yeah, no, ports are always extracted in a subdirectory of WRKDIR because other things get stored there.
If there are ports that break, they are probably doing something stupid and should be fixed.

It is not uncommon that a distfile does not contain a single subdir. We even have https://cgit.freebsd.org/ports/tree/Mk/bsd.port.mk#n460 for such cases. I think ${WRKDIR:H} is a better fit here.

fuz marked 2 inline comments as done.Mon, Dec 2, 11:24 PM
fuz added inline comments.
Mk/bsd.port.mk
1623

Using ${WRKDIR} is insufficient. If that was used, a git command executed in WRKDIR (not unlikely!) would traverse upwards to find the ports tree, as GIT_CEILING_DIRECTORIES is only queried when changing directories, but not when the working directory is checked for a .git directory. If you also want to ignore git repositories in WRKDIR, I can add both WRKDIR and WRKDIR:H to GIT_CEILING_DIRECTORIES.

No git command should be run in WRKDIR, ever. If a port does this, it should be fixed.

You somewhat assume that I don't understand what you mean, I do, I rewrote enough of the framework over the years to have an intimate knowledge of how it works, and how ports should work.
If a port gets broken because of this change, which is unlikely, that particular port should be fixed.

If you are unwilling to go through the hoops of working with portmgr when proposing a patch for the framework, please say so. Your change has value, and it should be included, but it should be included properly. If you do not want to change it as is required, I can take it over from here and finish the task. (Changing to ${WRKDIR} and going through with the exp-run.)

Mk/bsd.port.mk
1623

When a port's distfile does not create a subdirectory, it has to use NO_WRKSUBDIR, which magically creates a subdirectory for it, so WRKDIR is enough. The comment in there is somewhat wrong.

As I said, a port that does not do this it is broken and should be fixed.

1623

Can you please do as I ask ?

Mk/bsd.port.mk
1623

Ah, no, the comment is exactly right, I read it too fast.
When you set NO_WRKSUBDIR, the framework will magically create a WRKSRC based on the name of the package, and will extract inside WRKSRC, not inside WRKDIR, so a subdirectory is always present.

Hi @mat, I once again appreciate your time and reviews. It is my intention to justify the design choices I made and why I believe them to be important. Of course it's your final call to decide how things should be done and I respect that.

Mk/bsd.port.mk
1623

I just don't get why you want me to add an unnecessary hole to this failsafe. Any port that tries to run git to detect its version is already broken and clearly those do not get fixed on a regular basis as I keep seeing ports that do so. So I don't see how adding a hole to the failsafe is going to improve things. It just doesn't make sense to me. But if that is your wish, I will make the failsafe defective as you indicate.