radian-software / straight.el

🍀 Next-generation, purely functional package manager for the Emacs hacker.
MIT License
2.71k stars 150 forks source link

Specifying revisions, not just branches, in package recipes #246

Open nrvale0 opened 6 years ago

nrvale0 commented 6 years ago

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

raxod502 commented 6 years ago

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.

nrvale0 commented 6 years ago

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

raxod502 commented 6 years ago

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.

dieggsy commented 6 years ago

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.

raxod502 commented 6 years ago

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.

dieggsy commented 6 years ago

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

nrvale0 commented 6 years ago

... 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.

raxod502 commented 6 years ago

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).

mnewt commented 6 years ago

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.

raxod502 commented 6 years ago

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.

mnewt commented 6 years ago

@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!

raxod502 commented 6 years ago

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)))))
Fuco1 commented 5 years ago

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.

raxod502 commented 5 years ago

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?

FrauH0lle commented 5 years ago

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):

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.

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.

raxod502 commented 5 years ago

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.

conao3 commented 4 years ago

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.

raxod502 commented 4 years ago

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.

conao3 commented 4 years ago

OK, thanks!

jdek commented 4 years ago

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?

raxod502 commented 4 years ago

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.

jdek commented 4 years ago

@raxod502 so the lockfile pins dependency versions as well?

raxod502 commented 4 years ago

@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.

andreyorst commented 4 years ago

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?

raxod502 commented 4 years ago

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.

andreyorst commented 4 years ago

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!

raxod502 commented 4 years ago

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.

gtrak commented 4 years ago

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"

raxod502 commented 4 years ago

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).

gtrak commented 4 years ago

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.

raxod502 commented 4 years ago

it just shouldn't imply regular users have to fork

Assuredly not. As a regular user, even one who sometimes modifies package source code to play around with things, you do not need to fork anything. But, suppose you were to check out a pull request locally to see if it fixed a bug you were experiencing. When you later ask straight.el to ensure your configuration is reproducible, you would want it to tell you that you were on some random feature branch rather than master, right? This is why straight.el has the concept of a primary branch to be checked out. The question of which commit/tag along that branch is then a separate question.

gtrak commented 4 years ago

Will you validate that a specified commit is actually on a branch? I think how you're describing branch and commit are not really in line with git abstractions and it could cause conflicts. It sounds like your notion of branch automatically tracks an upstream remote branch?

I guess I can see a couple use-cases.

The default case, you're tracking a branch but pinning a particular commit automatically for reproducibility. In that case you specify a branch if it's not the repo default and do not specify a commit in the recipe.

In the manual pin case, you specify a commit sha, but there's no longer any need for a branch name and that can get confusing.

Since the term 'branch' is overloaded and, from a basic git perspective, it's surprising that the existing feature doesn't cover tags and arbitrary commits (a clone is a branch, after all), would it make more sense to call your current use of the word 'tracking-branch' or 'sync-branch' or something like that?

On Sun, Sep 20, 2020 at 9:53 AM Radon Rosborough notifications@github.com wrote:

it just shouldn't imply regular users have to fork

Assuredly not. As a regular user, even one who sometimes modifies package source code to play around with things, you do not need to fork anything. But, suppose you were to check out a pull request locally to see if it fixed a bug you were experiencing. When you later ask straight.el to ensure your configuration is reproducible, you would want it to tell you that you were on some random feature branch rather than master, right? This is why straight.el has the concept of a primary branch to be checked out. The question of which commit/tag along that branch is then a separate question.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/raxod502/straight.el/issues/246#issuecomment-695789820, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQFOUUUUEHMPVDFRFLC3DSGYCLZANCNFSM4ETL42PA .

raxod502 commented 4 years ago

Will you validate that a specified commit is actually on a branch?

The default behavior when checking out a repository with both a branch and commit specified is to fetch that commit and point the branch to it. Furthermore, repository normalization will ensure that the primary branch remains checked out, and the lockfile system handles synchronization in both directions.

I think how you're describing branch and commit are not really in line with git abstractions and it could cause conflicts.

Can you say more about this? I don't see any difference between what Git means by branch and what straight.el means by branch. It is true that in addition to enforcing a particular branch is checked out, straight.el also enforces that it has an appropriate relationship on the commit graph to a corresponding remote branch which must exist.

In the manual pin case, you specify a commit sha, but there's no longer any need for a branch name and that can get confusing.

I suppose this is true, but there is an advantage in terms of implementation simplicity/comprehensibility if we only support checking out a particular commit by means of git reset (which moves the branch pointer) and not by means of git checkout (which detaches HEAD).

it's surprising that the existing feature doesn't cover tags and arbitrary commits

I think it's reasonable for a keyword called :branch to only support branches. If it were going to support other things it could be called :ref instead, which would also be in line with Git terminology (all branches are refs, but not vice versa).

gtrak commented 3 years ago

Yea, a :ref would be great.