Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not update packages if no_update is set. #16

Closed
wants to merge 1 commit into from

Conversation

pmeiyu
Copy link

@pmeiyu pmeiyu commented Oct 31, 2018

Do not update packages if no_update is set.

@pmeiyu pmeiyu changed the title feat(install-packages.sh): Respect option_no_update. Do not update packages if no_update is set. Oct 31, 2018
@lotem lotem self-requested a review November 2, 2018 03:02
@lotem
Copy link
Member

lotem commented Nov 2, 2018

This change is incorrect.
update-package.sh has to be called to checkout the required branch.

What is the problem you are trying to solve with this change?

else
echo $(info 'Found package:') $(highlight "${package}")
fi
"${script_dir}"/update-package.sh "${package_dir}" "${branch}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no_update is also read in this script.

@@ -57,10 +57,10 @@ fetch_or_update_package() {
else
if [[ -z "${option_no_update}" ]]; then
echo $(info 'Updating package:') $(highlight "${package}")
"${script_dir}"/update-package.sh "${package_dir}" "${branch}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skipping this call could leave the working copy on a different branch.

@pmeiyu
Copy link
Author

pmeiyu commented Nov 2, 2018

I'm building rime-data on Guix. Guix does not allow updating source code when building packages. So plum should not try to update schema packages when no_update is set.

When building rime-data on Guix, every schema package is downloaded by Guix and it's limited to a specific commit. If a branch is needed, then that branch's commit is downloaded. Both updating git repository and switching git branch are against the principle of reproducible builds.

@lotem
Copy link
Member

lotem commented Nov 2, 2018

I understand your requirement.
With no_update=1 the script won't pull from remote unless the requested branch doesn't exist locally. You do not need this code change.

@lotem lotem closed this Nov 2, 2018
@pmeiyu
Copy link
Author

pmeiyu commented Nov 2, 2018

Currently plum throw a lot of warnings because it cannot find the git command. Guix building environment is limited to only necessary dependencies, so git is not available. Could this be fixed?

@lotem
Copy link
Member

lotem commented Nov 2, 2018

Does the build system support "dev/build dependencies"?
If you aim to package data, git sounds like a build time dependency but not required at runtime.

@pmeiyu
Copy link
Author

pmeiyu commented Nov 2, 2018

Yes I can pass git as a build time dependency, but plum still throw warnings because schema packages are not git repositories.
WARNING: not a git repository, skipped updating './package/rime/bopomofo'
Schema packages are pre-downloaded by Guix and are copied (symlink actually) into "package/rime" directory, so they are not git repositories.

@pmeiyu
Copy link
Author

pmeiyu commented Nov 2, 2018

I am porting Rime to Guix. If you are interested, please have a look at the package definition I wrote for rime-data:
https://github.com/meiyopeng/guix/blob/rime/gnu/packages/ibus.scm#L322

@lotem
Copy link
Member

lotem commented Nov 2, 2018

Wocow, it's written in lisp.
There is a problem for you: the build itself isn't "reproducible", and source code is not tagged:
rime/plum doesn't yet support "locking" the version of downloaded packages, neither by tags, submodule reference, nor by saving the git ref in a lock file.

@pmeiyu
Copy link
Author

pmeiyu commented Nov 3, 2018

There is a problem for you: the build itself isn't "reproducible", and source code is not tagged:

Yeah, I know. I reported the reproducibility issue to librime, but still have no answer.

rime/plum doesn't yet support "locking" the version of downloaded packages, neither by tags, submodule reference, nor by saving the git ref in a lock file.

My solution is to manually specify commit id for every schema package and copy them into "package/rime" directory. I have already successfully built rime on Guix. I made this pull request to suppress build process warnings when building rime-data. It would be great if you can provide an option to prevent plum from trying to download packages and check out branches. The same option will also be useful to NixOS, a similar OS to Guix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants