Opened 10 years ago

Closed 9 years ago

#44300 closed update (fixed)

salt: update to 2014.1.7

Reported by: cro (C. R. Oldham) Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch maintainer Cc: ap@…, ryandesign (Ryan Carsten Schmidt), nerdling (Jeremy Lavergne)
Port: salt

Description

Portfile attached. This port now retrieves sources from Git and includes LaunchItems for salt-minion, salt-master, and salt-syndic. The variants +master and +syndic will trigger the installation of these .plists.

Attachments (3)

Portfile (3.8 KB) - added by cro (C. R. Oldham) 10 years ago.
Portfile for Salt
main.log (42.0 KB) - added by cro (C. R. Oldham) 9 years ago.
'port install salt' log showing lack of reference to github port group and attempt to find distfiles at mirrors instead of from GitHub
Portfile-salt.diff (3.9 KB) - added by cro (C. R. Oldham) 9 years ago.
Diff updating portfile to install Salt 2014.1.13

Download all attachments as: .zip

Change History (16)

Changed 10 years ago by cro (C. R. Oldham)

Attachment: Portfile added

Portfile for Salt

comment:1 Changed 10 years ago by mf2k (Frank Schima)

Keywords: salt salt-master salt-minion saltstack removed
Priority: HighNormal
Version: 2.3.1

The Priority field is for use by Macports team members only.

Per the guidelines, please instead attach a unified diff of the Portfile so we can easily see what changes you are proposing.

comment:2 Changed 10 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ap@… added
Keywords: haspatch maintainer added
Summary: Portfile for Salt 2014.1.7salt: update to 2014.1.7

This ticket supersedes #44241.

The diff is reversed.

Did you consider making subports instead of variants as I suggested elsewhere?

Your hardcoded references to /opt/local need to be replaced with the ${prefix} variable.

Your post-destroot code that modifies things outside of the destroot cannot work; MacPorts will print a permission denied error. Setting up default configuration files is done in post-activate, not post-destroot. However, doing things in /etc is probably not desired. MacPorts ports should confine themselves to the MacPorts prefix.

comment:3 Changed 10 years ago by ryandesign (Ryan Carsten Schmidt)

The github portgroup should be used. If possible, fetch from the github-generated tarball instead of directly from the git repository. See comments in the github portgroup for instructions.

Why are any of the variants needed? All they seem to do is install additional files, which the user can use if desired. Why not just always install these files?

comment:4 Changed 10 years ago by cro (C. R. Oldham)

The diff is reversed.

Oops. Corrected.

Your hardcoded references to /opt/local need to be replaced with the ${prefix} variable.

Fixed.

Setting up default configuration files is done in post-activate

Fixed.

However, doing things in /etc is probably not desired.

Yeah, I thought so, that's why I just installed a symlink instead of copying the files directly to /etc. Salt isn't terribly forgiving right now about locating the config directory somewhere other than /etc.

Did you consider making subports instead of variants as I suggested elsewhere?

Yes, I tried that, but since the python installer just installs all the files, the subports would complain that the files already existed. In any case...

Why are any of the variants needed?

Good suggestion, I removed them.

The github portgroup should be used

Fixed. Thanks, I did not know the github portgroup could install from the github-generated tarfile.

comment:5 Changed 10 years ago by mf2k (Frank Schima)

Why do you add notes-append in a post-destroot block? It seems that you should simply use "notes" and not in any block.

comment:6 Changed 10 years ago by cro (C. R. Oldham)

Hmm..it seems Trac ate my comments. The Portfile diff I submitted 5 days ago updates Salt to 2014.1.10 and I think resolves all the issues that had been discussed in this thread. We're looking forward to having this available in MacPorts. Thanks!

comment:7 Changed 10 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign@… added

Thanks, but there are still numerous issues:

  • The patch does not apply. For example, the patch wants to delete a line "# Portfile for saltstack" but no such line is in the portfile. (That line was in the portfile that was submitted in #41191 but I did not commit that line because it didn't tell us anything we didn't already know.) Be sure you're generating your diff based on the latest version of the portfile that's currently in our repository.
  • You're fetching from git. Do you really need to? Usually we want to fetch from a tarball, which is what the github portgroup will do unless you specify otherwise. You shouldn't need to set git.url or git.branch or livecheck; running github.setup sets those for you. You may just need to add "v" as the 4th argument to github.setup. That's the tag prefix.
  • Is there any difference between the automatically created repository tag tarball and the manually uploaded release tarball? Possibly you may want to use the release tarball instead. If so, use "github.tarball_from releases"
  • If you're going to mess with the user's /etc in post-activate, you should undo it in pre-deactivate.
  • Instead of "file link", you should use "ln -s"
  • Instead of "file copy", you can just use "copy"
  • When you copy items into /Library/LaunchDaemons, please do it as part of the destroot phase, installing into ${destroot}/Library/LaunchDaemons, so that it will be cleaned up when the port is uninstalled
  • The second time you link /etc/salt to /opt/local/etc/salt seems to be unnecessary (and, besides, hardcodes /opt/local)

comment:8 Changed 9 years ago by ocroquette (Olivier Croquette)

See also #45656

comment:9 Changed 9 years ago by cro (C. R. Oldham)

Greetings,

I've updated the diff to address all the bullet points from the last cc (and I apologize for letting this lay dormant). The diff also creates a Portfile that will install 2014.1.13.

ocroquette@…: Thank you for the reminder in ticket #45656.

Changed 9 years ago by cro (C. R. Oldham)

Attachment: main.log added

'port install salt' log showing lack of reference to github port group and attempt to find distfiles at mirrors instead of from GitHub

comment:10 Changed 9 years ago by cro (C. R. Oldham)

Yeah, this is just weird.

I confirmed with my team that we do create a GitHub release with each official release, so I made sure one existed for 2014.1.13. I changed the Portfile to use 'github.tarball_from releases'. However, on some of my machines MacPorts still can't find a download.

When I go to GitHub and look at the download links the button link points to

https://github.com/saltstack/salt/archive/v2014.1.13.tar.gz

Clicking on that link does a redirect and eventually does this:

wget https://github.com/saltstack/salt/archive/v2014.1.13.tar.gz
--2014-11-13 11:04:57--  https://github.com/saltstack/salt/archive/v2014.1.13.tar.gz
Resolving github.com (github.com)... 192.30.252.130
Connecting to github.com (github.com)|192.30.252.130|:443... connected.
HTTP request sent, awaiting response... 302 Found
Location: https://codeload.github.com/saltstack/salt/tar.gz/v2014.1.13 [following]
--2014-11-13 11:04:57--  https://codeload.github.com/saltstack/salt/tar.gz/v2014.1.13
Resolving codeload.github.com (codeload.github.com)... 192.30.252.147
Connecting to codeload.github.com (codeload.github.com)|192.30.252.147|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 2872271 (2.7M) [application/x-gzip]
Saving to: ‘v2014.1.13.tar.gz’

v2014.1.13.tar.gz          100%[=========================================>]   2.74M   329KB/s   in 7.6s

2014-11-13 11:05:05 (369 KB/s) - ‘v2014.1.13.tar.gz’ saved [2872271/2872271]

so am I missing something obvious or did GitHub change the way they serve up these releases?

comment:11 Changed 9 years ago by cro (C. R. Oldham)

I'm retracting the above. I figured it out after reading the docs at the top of the github portgroup definition file more carefully. I've updated the Portfile diff to one that has worked for me everywhere that I have tested it.

Changed 9 years ago by cro (C. R. Oldham)

Attachment: Portfile-salt.diff added

Diff updating portfile to install Salt 2014.1.13

comment:12 Changed 9 years ago by nerdling (Jeremy Lavergne)

Cc: snc@… added

Cc Me!

comment:13 Changed 9 years ago by nerdling (Jeremy Lavergne)

Resolution: fixed
Status: newclosed

Updated in r128431.

Note: See TracTickets for help on using tickets.