Page MenuHomeSolus

Update pacman to 0.9.4
Needs ReviewPublic

Authored by Jacalz on Fri, Feb 1, 7:51 AM.

Details

Reviewers
JoshStrobl
Group Reviewers
Triage Team
Summary

There are no new release tags but they have updated the version in the commit history a couple times, so I made sure to fetch the latest commit.

Changelog:
No changelog provided by upstream

Packaging Changes:

  • Renamed desktop entry from Pacman (SDL) to Pacman
  • Improved the description and summary
Test Plan
  • Played a couple games of Pacman
  • Verified that the .desktop file gets installed and works as i should.

Diff Detail

Repository
R2284 pacman
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Jacalz created this revision.Fri, Feb 1, 7:51 AM
Jacalz requested review of this revision.Fri, Feb 1, 7:51 AM
Jacalz updated this revision to Diff 12724.

Fix empty line on top of package.yml

Jacalz retitled this revision from Update pacman to 0.9.4 and add ugly hack to fix the build to Update pacman to 0.9.4 and hack to fix the build.Thu, Feb 7, 6:50 AM
Jacalz edited the summary of this revision. (Show Details)
Jacalz edited the test plan for this revision. (Show Details)
JoshStrobl requested changes to this revision.Fri, Feb 8, 3:54 PM
JoshStrobl added a subscriber: JoshStrobl.
JoshStrobl added inline comments.
package.yml
16–18

What is the purpose of this. All those files exist.

This revision now requires changes to proceed.Fri, Feb 8, 3:54 PM
Jacalz added inline comments.Fri, Feb 8, 4:00 PM
package.yml
16–18

I know, but the build just gives errors and won't build without using this. I know it is really weird but it was the only solution that I could find.

JoshStrobl added inline comments.Fri, Feb 8, 4:01 PM
package.yml
16–18

What were the errors?

Jacalz added inline comments.Fri, Feb 8, 4:13 PM
package.yml
16–18

I don’t have time to test right now but I am pretty certain that they were about aclocal missing from system. I did a lot of research and found out that aclocal is part of autotools and I found a thread somewhere with someone having the same issue as me. The fix provided was what I provided here.

Jacalz added a comment.Sat, Feb 9, 2:38 PM

Here is the log over the error that keeps me from building the package: @JoshStrobl

CDPATH="${ZSH_VERSION+.}:" && cd . && /bin/sh /home/build/YPKG/root/pacman/build/pacman.git/missing aclocal-1.15 
/home/build/YPKG/root/pacman/build/pacman.git/missing: line 81: aclocal-1.15: command not found
WARNING: 'aclocal-1.15' is missing on your system.
         You should only need it if you modified 'acinclude.m4' or
         'configure.ac' or m4 files included by 'configure.ac'.
         The 'aclocal' program is part of the GNU Automake package:
         <http://www.gnu.org/software/automake>
         It also requires GNU Autoconf, GNU m4 and Perl in order to run:
         <http://www.gnu.org/software/autoconf>
         <http://www.gnu.org/software/m4/>
         <http://www.perl.org/>
Jacalz updated this revision to Diff 12951.Sun, Feb 10, 7:20 AM
  • Improve the description and summary
xulongwu4 added a subscriber: xulongwu4.EditedSun, Feb 10, 1:33 PM

@Jacalz This might help you: https://stackoverflow.com/questions/33278928/how-to-overcome-aclocal-1-15-is-missing-on-your-system-warning

It suggests the configure script might be out of date when you checkout code from github instead of extracting it from a tarball. That's why we need to touch configure. Running autoreconf -f -i before ./configure can also solve the problem.

Jacalz updated this revision to Diff 12963.Sun, Feb 10, 5:12 PM
  • Replace hack with a more sane approach and patch the desktop file

Thanks for the tip @xulongwu4, it is a much better solution than my ugly hack. 👍
I also made sure to patch the .desktop file to rename it from Pacman (SDL) to Pacman, since the former just confuses people. The latter is also the name of the application so it makes sense from an end user perspective.

Jacalz retitled this revision from Update pacman to 0.9.4 and hack to fix the build to Update pacman to 0.9.4.Sun, Feb 10, 5:17 PM
Jacalz edited the summary of this revision. (Show Details)
Jacalz edited the test plan for this revision. (Show Details)
JoshStrobl requested changes to this revision.Mon, Feb 18, 6:00 AM
JoshStrobl added inline comments.
package.yml
17

This isn't necessary, our %reconfigure macro does this and %configure. See https://github.com/getsolus/ypkg/blob/master/ypkg2/rc.yml#L5

This revision now requires changes to proceed.Mon, Feb 18, 6:00 AM
Jacalz updated this revision to Diff 13149.Mon, Feb 18, 9:32 AM
  • Use %reconfigure variable instead of autoreconf
Jacalz marked an inline comment as done.Mon, Feb 18, 9:34 AM
Jacalz added inline comments.
package.yml
17

Didn't know that it could do that, you learn something new every day. Thanks 👍