Page MenuHomeSolus

Update pacman to 0.9.4
ClosedPublic

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Jacalz created this revision.Feb 1 2019, 7:51 AM
Jacalz requested review of this revision.Feb 1 2019, 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.Feb 7 2019, 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.Feb 8 2019, 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.Feb 8 2019, 3:54 PM
Jacalz added inline comments.Feb 8 2019, 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.Feb 8 2019, 4:01 PM
package.yml
16–18

What were the errors?

Jacalz added inline comments.Feb 8 2019, 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.Feb 9 2019, 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.Feb 10 2019, 7:20 AM
  • Improve the description and summary
xulongwu4 added a subscriber: xulongwu4.EditedFeb 10 2019, 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.Feb 10 2019, 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.Feb 10 2019, 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.Feb 18 2019, 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.Feb 18 2019, 6:00 AM
Jacalz updated this revision to Diff 13149.Feb 18 2019, 9:32 AM
  • Use %reconfigure variable instead of autoreconf
Jacalz marked an inline comment as done.Feb 18 2019, 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 👍

JoshStrobl requested changes to this revision.Mar 15 2019, 10:42 AM
JoshStrobl added inline comments.
files/drop-SDL-from-desktop-entry.patch
21 ↗(On Diff #13149)

If we're patching the name, we may as well remove the sed call on ln17 of the package.yml and just patch the desktop file to change /usr/local/ to just /usr/

This revision now requires changes to proceed.Mar 15 2019, 10:42 AM
Jacalz updated this revision to Diff 13953.Mar 20 2019, 7:22 PM
Jacalz marked an inline comment as done.
  • Patch desktop entry instead of using sed
DataDrake accepted this revision.Mar 22 2019, 9:17 PM
DataDrake added a subscriber: DataDrake.

LGTM. Thanks!

This revision was not accepted when it landed; it landed in state Needs Review.Mar 22 2019, 9:19 PM
Closed by commit R2284:73d2a31e2e50: Update pacman to 0.9.4 (authored by Jacalz, committed by DataDrake). · Explain Why
This revision was automatically updated to reflect the committed changes.