FS#9230 - makepkg should check makedepends before checking VCS version

Attached to Project: Pacman
Opened by Xilon (Xilon) - Wednesday, 16 January 2008, 15:30 GMT
Last edited by Dan McGee (toofishes) - Friday, 08 February 2008, 02:22 GMT
Task Type Bug Report
Category makepkg
Status Closed
Assigned To Dan McGee (toofishes)
Architecture All
Severity Low
Priority Normal
Reported Version 3.1.0
Due in Version 3.1.2
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

I couldn't find this reported but it'll probably end up as a dupe like all the others I submitted :P

Summary and Info:
makepkg doesn't check whether the VCS system (subversion, git, mercurial, etc) is installed before trying to update the pkgver. It should at least check dependencies before doing so.

Steps to Reproduce:
Try to build any VCS package without the necessary VCS installed

$ makepkg
==> Determining latest hg revision...
/usr/bin/makepkg: line 1012: hg: command not found
This task depends upon

Closed by  Dan McGee (toofishes)
Friday, 08 February 2008, 02:22 GMT
Reason for closing:  Fixed
Additional comments about closing:  Fixed in commit 14ee1be1ef21e09a84f01dc9623e8513218709d4
Comment by Dan McGee (toofishes) - Wednesday, 16 January 2008, 15:45 GMT
Good catch. Want to look into a patch for this?
Comment by Xavier (shining) - Sunday, 20 January 2008, 11:52 GMT
Could it be possible to simply move the depends / makedepends check just before the devel_check / devel_update lines?
Unfortunately, I am not familiar enough with makepkg yet to take the responsibility for this change :)
Comment by Xilon (Xilon) - Sunday, 20 January 2008, 16:43 GMT
It seems that would result in having the dependency installation occur within a fakeroot environment, which makepkg seems to avoid. If I knew the code better I'd have a crack at it, but it seems too tricky.
Comment by Scott H (stonecrest) - Monday, 28 January 2008, 03:15 GMT
I don't think it needs to be as complicated as checking makedepends. I've attached a simple patch that fixes this by merely bypassing the devel_check. In these situations, it will cause makepkg to fail as so:

$ makepkg
==> Making package: mpd-svn 7168-1 (Sun Jan 27 20:12:37 MST 2008)
==> Checking Runtime Dependencies...
==> Checking Buildtime Dependencies...
==> Missing Dependencies:
-> subversion
==> ERROR: Could not resolve all dependencies.
   makepkg.patch (1.5 KiB)
Comment by Xilon (Xilon) - Monday, 28 January 2008, 06:21 GMT
Very clever. I don't really like how the VCS executable is hard coded, but since we only support specific VCS's anyway, I guess it's not so bad.
Comment by Xilon (Xilon) - Monday, 28 January 2008, 06:23 GMT
You can't edit comments? :(
Well, I wanted to add that for the VCS checks that don't actually use the executable (but instead date, etc), the check can still go through.
Comment by Xavier (shining) - Monday, 28 January 2008, 07:39 GMT
I thought about that way too, it's just that it duplicates the work a bit.
makepkg will first check if the executable is available, and then check if the package is available while checking make dependencies.
But well, I guess it's fine, and it's probably simpler that way.
And I like having the VCS checks done in all case (even when date is used). I find it more consistent.
Comment by Dan McGee (toofishes) - Wednesday, 06 February 2008, 00:33 GMT
  • Field changed: Due in Version (Undecided → 3.1.2)
+ if ! type darcs >/dev/null 2>&1; then
+ return
+ fi

No error message if we are missing the executable? I guess that seems weird but you do point out above it should be listed in the makedepends.

We could probably go with something like this if we don't need error messages:
type darcs >/dev/null 2>&1 || return

which would not scream "refactor me!" near as much as the above copy/paste stuff did.

Let's shoot for 3.1.2 with this one.
Comment by Xavier (shining) - Wednesday, 06 February 2008, 06:33 GMT
Indeed, we don't return any error here, which would be problematic if the executable wasn't listed in the makedepends.
However, it has always been that way. If the correct depends / makedepends aren't correctly listed, and are not installed, the build will fail. I don't think there is much we can do about that.
Besides, having the correct makedepends is more practical for the user, because makepkg can install these automatically.

Finally, this little fix seems appropriate for 3.1.2 (we could also change these 3 lines block to just one line indeed).
We can always try moving the makedepends check before devel check for 3.2.

Loading...