straight.el icon indicating copy to clipboard operation
straight.el copied to clipboard

Specifying revisions, not just branches, in package recipes

Open nrvale0 opened this issue 6 years ago • 34 comments

:branch works with other refs like a tag name. Which is great. But perhaps it should be renamed to :ref?

nrvale0 avatar Mar 03 '18 16:03 nrvale0

This requires some more thought. The repository management algorithms are not actually designed to work with any refs other than branch names, since it's expected that you use version lockfiles to specify exact revisions or tags, rather than putting them in the recipes directly.

raxod502 avatar Mar 03 '18 17:03 raxod502

Ah, ok. I've just gotten started with straight.el so I've not yet explored lockfiles. Entirely possible this is better approach.

nrvale0 avatar Mar 03 '18 18:03 nrvale0

If your goal in specifying a tag or revision in the recipe is that your configuration will use that revision of the package, then indeed version lockfiles are designed to achieve exactly that purpose.

raxod502 avatar Mar 03 '18 21:03 raxod502

That said, it'd be interesting to have a :ref that supports arbitrary refs, with the end goal being that the only source of truth is the init file. This is probably not as intuitive as lockfiles since they're derived from whatever ref you git checkout to, but it might be a nice advanced feature.

dieggsy avatar Mar 04 '18 03:03 dieggsy

it'd be interesting

Sure, and I wouldn't be opposed to this...

with the end goal being that the only source of truth is the init file

... but I'm honestly not sure why you'd want to do it. You'd have to edit your init-file every time you update a package.

raxod502 avatar Mar 04 '18 04:03 raxod502

Well, if your workflow is one in which you update a lot (which I actually do) then it'd be annoying, but some people might prefer to "pin" packages in that way, esp. if they don't update frequently or want to choose their own definition of "stable enough".

Regardless, it'd be an additional feature, so you wouldn't be required to 'pin' every package with a specific ref throughout. But if a package came up that you did want to pin or revert, you'd have the convenience of simply specifying the ref in the init file, without pinning everything to its current state through straight-freeze-versions

dieggsy avatar Mar 04 '18 04:03 dieggsy

... but I'm honestly not sure why you'd want to do it. You'd have to edit your init-file every time you update a package.

I can speak to this a bit. I went down the path of straight.el to allow me to install the latest version of Org and associated packages without conflicts with RPM-packaged Org on Fedora (not possible with use-package). All other packages are still installed with use-package.

So, in my case, having a lockfile separate from the package specification in my init-file is just one more thing to track in the repo and in my head with not much payout.

Though I agree, having worked with similar setups in other environments (requirements.txt, Gemfile.lock, etc) that specification in the init-file probably does not scale beyond a handful of packages under straight.el management.

nrvale0 avatar Mar 04 '18 15:03 nrvale0

Regardless, it'd be an additional feature

Fair enough, I see no reason why this can't coexist with the current functionality.

that specification in the init-file probably does not scale

Indeed, especially once you consider that not only do you have to add a revision manually for every package that is mentioned in your init-file, but also all of their dependencies (which can only be determined by looking at the source code of those packages), as well as any dependencies that might be added when you update a package. This would need tooling to be manageable (not that there is anything wrong with that).

raxod502 avatar Mar 04 '18 18:03 raxod502

I have a use case, which is similar to those discussed but perhaps with a little different nuance. I usually update all my packages at once, with these commands:

straight-normalize-all
straight-fetch-all
straight-merge-all
;; verify that things are working
straight-freeze-versions

For most packages, that works great. However, I have a couple packages I don't want to update, but instead pin at a known good release or commit. So, for most packages, I'm happy to write a straight/versions/default.el file out without having to micro-manage each revision but would like to only explicitly pin certain packages with :ref or similar.

Would the current way be to use the profile system?

This proposal seems the best way forward to me.

mnewt avatar Aug 21 '18 23:08 mnewt

The interaction of the profile system and lockfiles needs some more work, see e.g. #67. Currently there isn't really any interaction.

I agree that it would be nice if we could support your use case using an existing feature. Some thought would be required to determine if this is possible. I don't necessarily have an objection to implementing a separate pinning system, as long as the implications are well-thought-out.

As a workaround, you could simply write your own version of straight-merge-all which skips packages in your list:

(defun my-straight-merge-unpinned (&optional from-upstream predicate)
  "Try to merge all packages from their primary remotes.
With prefix argument FROM-UPSTREAM, merge not just from primary
remotes but also from configured upstreams.

Return a list of recipes for packages that were not successfully
merged. If multiple packages come from the same local
repository, only one is merged.

PREDICATE, if provided, filters the packages that are merged. It
is called with the package name as a string, and should return
non-nil if the package should actually be merged.

PREDICATE is automatically modified so that packages in the list
`my-pinned-packages' are not merged."
  (interactive "P")
  (straight--map-existing-repos-interactively
   (lambda (package)
     (straight-merge-package package from-upstream))
   (lambda (package)
     (and (not (member package my-pinned-packages))
          (or (null predicate) (funcall predicate package))))))

Warning: I have not tested this code.

raxod502 avatar Aug 22 '18 16:08 raxod502

@raxod502 I just tested your code and it works perfectly. When I run the straight-*-all and straight-freeze-versions commands, the packages in the my-pinned-packages list are not updated. This is a good workaround and anyone else with this use case can simply copy and paste.

Many thanks!

mnewt avatar Aug 22 '18 23:08 mnewt

It's not clear why I failed to think of this earlier, but you can easily do this without duplicating the logic of straight-merge-all and without using internal functions:

(defun my-straight-merge-all (&optional from-upstream)
  "Try to merge all packages from their primary remotes.
With prefix argument FROM-UPSTREAM, merge not just from primary
remotes but also from configured upstreams.

Do not merge packages listed in `my-pinned-packages'."
  (interactive "P")
  (straight-merge-all
   from-upstream
   (lambda (package)
     (not (member package my-pinned-packages)))))

raxod502 avatar Aug 26 '18 23:08 raxod502

FWIW there is a usecase for

You'd have to edit your init-file every time you update a package.

I've been using some 5 years old helm version because I'm only using it for one specific source and newer versions break it. So I just added it as a git submodule to my config for now (I have my own fork). This is something I will never update since it works and I'm otherwise not a helm user.

Fuco1 avatar Jan 03 '19 10:01 Fuco1

Nevertheless, would it not be a superior user experience to simply mark Helm as pinned, rather than copying and pasting the commit hash into your configuration file and maintaining a submodule?

raxod502 avatar Jan 03 '19 19:01 raxod502

I hope it fine if I just jump into this thread as I did want to open up a new issue.

It is probably my own inexperience but I find the way straight.el handles, or is supposed to handle, package versions confusing.

So far, this is what I understand (please correct me if I am wrong):

  • package.el together with use-package supports the :pin keyword, thus one can pin the version of a package to melpa-stable. straight.el does not support that.
  • straight.el allows the use of a :branch keyword, which takes only branch names but not tags or commits as an argument. Thus, specifying :branch "release_9.2.1" will not pin the org package to this release (at least when I tried that).
  • straight-freeze-versions generates a version lock file which writes the currently used commit of all packages under straight's control into a file. Supports multiple files via straight's profile system. If present during initial install, this file will dictate the used package versions.
  • Here and here the workflow to fix package versions is described as

The expected workflow for updating packages is to update the packages manually or using one of the interactive commands, and then to regenerate (overwrite) the lockfile.

Checking out a particular revision should be done manually using git checkout or git reset.

This does make sense for me, however I think this would get very tedious for large package collections.

  • Here is a way to tell straight.el to ignore specified packages during merging, which allows to keep some packages pinned and the rest "rolling". It works but I encountered this issue. As soon as I invoke straight-freeze-versions, the lock file will contain the latest commit of the master branch and not the one checked out before.
  • From this thread here and others I understand that a :pin feature is planned but simply not implemented yet.

So I spent some time today in emacs and thanks to the power of open source and copy -> paste I came up with the following clumsy solution (as a minimal init.el) which might be of interest for others here in this thread.

(require 'cl-lib)
(require 'map)
(require 'subr-x)

(setq straight-repository-branch "develop")

(defvar bootstrap-version)
(let ((bootstrap-file
       (expand-file-name "straight/repos/straight.el/bootstrap.el" user-emacs-directory))
      (bootstrap-version 5))
  (unless (file-exists-p bootstrap-file)
    (with-current-buffer
        (url-retrieve-synchronously
         "https://raw.githubusercontent.com/raxod502/straight.el/develop/install.el"
         'silent 'inhibit-cookies)
      (goto-char (point-max))
      (eval-print-last-sexp)))
  (load bootstrap-file nil 'nomessage))

(straight-use-package 'git)

(defun org-git-version ()
  "The Git version of org-mode.
Inserted by installing org-mode or when a release is made."
  (require 'git)
  (let ((git-repo (expand-file-name
                   "straight/repos/org/" user-emacs-directory)))
    (string-trim
     (git-run "describe"
              "--match=release\*"
              "--abbrev=6"
              "HEAD"))))

(defun org-release ()
  "The release version of org-mode.
Inserted by installing org-mode or when a release is made."
  (require 'git)
  (let ((git-repo (expand-file-name
                   "straight/repos/org/" user-emacs-directory)))
    (string-trim
     (string-remove-prefix
      "release_"
      (git-run "describe"
               "--match=release\*"
               "--abbrev=0"
               "HEAD")))))

(provide 'org-version)

(defvar +straight-pinned-packages nil
  "List of pinned packages.")

;; Add `pinned' profile. Make sure that pinned is the last profile in straight-profiles.
(add-to-list 'straight-profiles '(pinned . "pinned.el") t)

(defun +straight-freeze-pinned-versions ()
  "Write lock file for pinned packages."
  (interactive)
  (let ((lockfile-path (straight--versions-lockfile 'pinned)))
    (with-temp-file lockfile-path
      (insert
       (format "(%s)\n:saturn\n"
               (mapconcat
                (apply-partially #'format "%S")
                +straight-pinned-packages
                "\n "))))))

(defun +straight--get-pinned-versions ()
  "Read pinned version lockfiles and return merged alist of saved versions.
The alist maps repository names as strings to versions, whose
interpretations are defined by the relevant VC backend."
  (let ((versions nil))
    (dolist (spec '((pinned . "pinned.el")))
      (cl-destructuring-bind (_profile . versions-lockfile) spec
        (let ((lockfile-path (straight--versions-file versions-lockfile)))
          (when-let ((versions-alist (ignore-errors
                                       (with-temp-buffer
                                         (insert-file-contents-literally
                                          lockfile-path)
                                         (read (current-buffer))))))
            (dolist (spec versions-alist)
              (cl-destructuring-bind (local-repo . commit) spec
                (setq versions (straight--alist-set
                                local-repo commit versions))))))))
      versions))


(defun +straight-thaw-pinned-versions ()
  "Read pinned version lockfiles and restore package versions to
those listed."
  (interactive)
  (let ((versions-alist (+straight--get-pinned-versions)))
    (straight--map-repos-interactively
     (lambda (package)
       (let ((recipe (gethash package straight--recipe-cache)))
         (when (straight--repository-is-available-p recipe)
           (straight--with-plist recipe
               (type local-repo)
             ;; We can't use `alist-get' here because that uses
             ;; `eq', and our hash-table keys are strings.
             (when-let ((commit (cdr (assoc local-repo versions-alist))))
               (straight-vc-check-out-commit
                type local-repo commit)))))))))


(defun +straight-pull-all-unpinned ()
  "Pull all packages and restore pinned package versions."
  (interactive)
  (straight-pull-all)
  (message "Taking care of pinned versions ...")
  (+straight-thaw-pinned-versions)
  (message "Done!"))

(defun +straight-freeze-versions ()
  "Freeze all package versions but respect pinned packages."
  (interactive)
  (straight-freeze-versions)
  (+straight-freeze-pinned-versions))

(let ((straight-current-profile 'pinned))
  (straight-use-package 'org-plus-contrib)
  (straight-use-package 'org)
  (add-to-list '+straight-pinned-packages
               '("org" . "55963cb80376df8a620431e224aae2a756f5d7e4")))

The idea is to have a variable +straight-pinned-packages which has the same structure as a lockfile and a profile for the pinned packages.

+straight-freeze-pinned-versions will write this lockfile to the disk.

+straight--get-pinned-versions and +straight-thaw-pinned-versions are slight modifications of the original straight functions and do the same as the original functions but use only the pinned profile.

+straight-pull-all-unpinned should update all packages but then revert the pinned packages to their specified commit. If straight-pull-all is invoked before, +straight-pull-all-unpinned will reset the pinned packages.

+straight-freeze-versions should freeze all packages and then simply overwrite the pinned lockfile with the specified commits.

During my tests I could jump back and forth between the org master branch and several specified tags and commits. Maybe this is also useful for somebody else.

FrauH0lle avatar Mar 11 '19 21:03 FrauH0lle

Hmm… seems to me like the long-run solution is to fix #66 and then invoking M-x straight-merge-all would not be messing up the revisions in the first place.

I think a better workaround would perhaps be to iterate over all the affected Git repositories after invoking M-x straight-thaw-versions and reset master to (currently-detached) HEAD. Then M-x straight-merge-all and M-x straight-freeze-versions should work as expected.

In any case, thanks for your contribution. If you think this code would be usable by/useful to others, I have no objection to its inclusion in straight-x.el.

raxod502 avatar Mar 14 '19 04:03 raxod502

cc @arkhan.

What status of this issue? I've received issue that asks me to support version-specific install via :straight keyword for leaf (yet atnother use-package). If you implement straight-use-package handle :ref keyword, we can use this feature with no changes from leaf.

conao3 avatar Jan 25 '20 17:01 conao3

It could be implemented if done carefully, and I would accept such a contribution. I have not seen any work in this direction, however, probably since it does not add a lot on top of the existing lockfile system.

raxod502 avatar Jan 26 '20 17:01 raxod502

OK, thanks!

conao3 avatar Jan 26 '20 17:01 conao3

Maybe related, not sure if I would need to make a separate issue: I'd like to associate each of my packages inside my init.el with their hash instead of in a separate lockfile. Would this be included in this feature request?

jdek avatar Feb 18 '20 15:02 jdek

Yes, although it's not clear to me how this ability would be helpful. After all, even if you did that, all the versions of your dependencies would still be totally free.

raxod502 avatar Feb 18 '20 15:02 raxod502

@raxod502 so the lockfile pins dependency versions as well?

jdek avatar Feb 18 '20 16:02 jdek

@jdek Yes, otherwise it wouldn't be very much of a lockfile. The idea of straight.el is a configuration that is 100% reproducible unless the code disappears from upstream.

raxod502 avatar Feb 19 '20 15:02 raxod502

Yes, although it's not clear to me how this ability would be helpful.

Sometimes this really is helpful. Currently I have to go into package directory and checkout to desired commit, because with recent commits a bug was introduced and I'm not sure if it will be fixed any time soon, though I've reported the issue.

So my use case would be like this:

(use-package some-package-with-bug-in-recent-commits
  :straight (:host github
             :repo "package/repo"
             :ref "hashofthepackagewherethebugdidn'toccurr"))

Currently my only option is to use :branch keyword and checkout to some tag, but the last tag was a very long time ago, so I actually miss a lot of features that were introduced since, it's just the recent commit which broke my workflow that I want to avoid until the bug is fixed. This can be done through lockfile, but a bit overkill when you occasionally need this for one package that broke recently

After all, even if you did that, all the versions of your dependencies would still be totally free.

Many packages are self-contained, so this is not a problem.

The idea of straight.el is a configuration that is 100% reproducible unless the code disappears from upstream.

I know that many users commit their entire .emacs.d with all packages into repository, so when they pull it, they have 100% working setup. Since straight clones repositories, this means that packages will be added as submodules?

andreyorst avatar Jun 26 '20 10:06 andreyorst

This can be done through lockfile, but a bit overkill when you occasionally need this for one package that broke recently

My position, at least until I am convinced otherwise, is that having a lockfile is the correct solution to this problem, and it is not worth supporting an inferior workaround. If there is some reason that the lockfile system is difficult to use for this case, then I think it should be improved. Perhaps, for example, by making it very easy to roll back only a single package rather than all of them at the same time.

Then, we can add a keyword to the recipe format that allows you to disable updates for a package. So, if you upgrade and find a package broken, you can ask straight.el to roll it back to the last known working version in your configuration, and then you can set it to be "pinned". Then there is no duplication of the commit hash between your init-file and the lockfile.

We currently have a workaround for this feature implemented by #380.

Many packages are self-contained, so this is not a problem.

In practice this is often true, but I think that the smaller we can make the probability of unexpected breakage, the better, especially for new users. It would be ideal to establish best practices that ensure reproducibility in all cases, not just those where it is especially important.

Since straight clones repositories, this means that packages will be added as submodules?

Yes, indeed. See also Borg, which mandates this approach and provides fewer features than straight.el, preferring to let you manage packages with more control and deliberation.

raxod502 avatar Jun 26 '20 13:06 raxod502

Then there is no duplication of the commit hash between your init-file and the lockfile.

Maybe there was misunderstanding, but in my https://github.com/raxod502/straight.el/issues/246#issuecomment-650110306 I didn't intend for commit hash duplication between init file and the lockfile, because I meant that user init file acts as lock file

I'll look into #380, thanks!

andreyorst avatar Jun 26 '20 14:06 andreyorst

I meant there is no possibility for duplication as commit hashes can be placed in one and exactly one location. It means that the mental model and implementation are both simpler.

raxod502 avatar Jun 27 '20 12:06 raxod502

Pinning isn't quite the same as specifying a particular commit/release. It's about deciding when to take on the risk of any particular update. I have an existing config with existing versions when moving from another tool to straight.el, first it was el-get, then use-package+quelpa, now use-package+straight. Since I use a lot of different language integrations of varying quality, I want control over when I have to learn a new one. Even if dependencies are still moving around freely, that's usually less of a problem, and if it happens to break one of my pins, I'm happy to update just that one at a particular time. What I don't want to do is break everything all at once and have to fix it before I can do any work again.

Generally I like to separate the versions I want for a particular reason vs just the version locks for reproducibility's sake. Manual pins are constraints on version resolution, lockfiles are a snapshot of a resolved set of dependencies. I don't want to have to keep in mind which are which, so it would be nice to separate them in the config I maintain somehow.

I do this in npm and python with pip-compile: https://github.com/jazzband/pip-tools#without-setuppy You could and we do pin a particular dep version, but also allow its transitive deps to upgrade themselves, given all the respective version constraints are satisfied.

I tried to use the branch feature for tags, I don't see why straight has to differentiate between tags and branches. Branching policies are different repo to repo and not inherent to 'git'. I wouldn't expect the branch feature to ever be useful unless it can handle other refs. Warning (straight): Could not check out branch "v0.7.0" of repository "lsp-python-ms" Warning (straight): Could not check out branch "v0.19.0" of repository "cider" Warning (straight): Could not check out branch "v2.90.1" of repository "magit"

gtrak avatar Sep 03 '20 15:09 gtrak

Generally I like to separate the versions I want for a particular reason vs just the version locks for reproducibility's sake

Yes, this is a good idea. I think what I suggest in https://github.com/raxod502/straight.el/issues/246#issuecomment-650188900 (lockfile + manual pins in the recipe format for particular packages) would achieve what you want. Perhaps we could add the extension that you can optionally specify the commit in the recipe as well, and that would unconditionally force the lockfile to match, rather than just keeping straight.el from making any changes to what version is checked out when you do an upgrade, etc.

I do this in npm and python with pip-compile

Sure, or with Pipenv, or Poetry, or NPM, or Yarn. What you outline is I think generally accepted as The Right Way To Do Things(tm).

I don't see why straight has to differentiate between tags and branches

This is because unlike the MELPA build system, straight.el actually supports interactive development. That means it needs a concept of which branch is supposed to be checked out, so that when you ask straight.el to normalize all your repository configuration, it can check out the appropriate branch and say intelligent-sounding things when detecting you've added commits locally that should be pushed, or similar things.

To be clear, I'm not opposed to adding the ability to pin a tag, but it's a conceptually different thing than setting the branch, because branches are much more powerful and complex than tags (they are used for more things than just having a convenient name to refer to a commit hash).

raxod502 avatar Sep 16 '20 13:09 raxod502

yes, (lockfile + manual pins in the recipe format for particular packages) sounds great, but it definitely needs the ability to specify a particular revision.

re: interactive development, that sounds like a use-case for library writers and not regular users. It seems fine, it just shouldn't imply regular users have to fork a bunch of packages they don't intend to modify.

gtrak avatar Sep 16 '20 15:09 gtrak