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

Introduce support for upgrading dependencies; Change mbedtls_url to the Github source; Make Shadowsocks-libev be the preferred option #119

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

IceCodeNew
Copy link
Contributor

  1. Introduce support for upgrading dependencies.
    0.1. Upgrading dependencies while reinstalling(which is actually means upgrading) shadowsocks.
  2. Make Shadowsocks-libev be the preferred option.
  3. Change mbedtls_url to the Github source.
    2.1. Adapt for archive structures from GitHub source.

在随后的提交中,我将提出一个在我看来更好的策略处理这个问题。
并不是非要坚持用我的办法来解决问题,关键在于这一提交和后来的多个提交都产生了严重的冲突,解决冲突实在太花时间花精力了。
This reverts commit 1e25c70.
@IceCodeNew
Copy link
Contributor Author

@teddysun 嗨,等你有空的时候可以麻烦给点建议或者合并 PR 吗?

@teddysun
Copy link
Owner

teddysun commented Jul 6, 2020

PR 里一次有太多更改,并不方便 review
建议分开来,先从 [] 改为 [[]] 等微小的部分开始吧……

另,

  1. libsodium 的源保持为 Github 比较好。
  2. 函数名func_decision_with_TRUE_or_FALSE_as_default_value不觉得太长了么?
  3. https://github.com/teddysun/shadowsocks_install/pull/119/files#diff-88e41fcd701c83c7270ce4ecf3a63deaR345 处类似的是什么?
  4. 安装时,选取的顺序不推荐更改,请保持原状。

use `[[ … ]]` rather than `[ … ]`
(refer: google.github.io/styleguide/shellguide.html#s6.3-tests)
Upgrading dependencies while reinstalling(which is actually means upgrading) shadowsocks.
0. Change `mbedtls_url` to the Github source
1. fix SC2181
2. Also comes with some minor fix
…ry limited memory

In case someone is trying to install shadowsocks on a machine with very limited memory
@IceCodeNew
Copy link
Contributor Author

@teddysun

PR 里一次有太多更改,并不方便 review
建议分开来,先从 [] 改为 [[]] 等微小的部分开始吧……

我非常同意你说的这一点。抱歉,这个提交应该是重新开 PR 前你要求我不要将 wget --no-check-certificate 改成 $(type -P wget)
所以我不得不利用 git stash 和 rebase 来修改提交历史,然后意外把一个不应该使用的本地提交放上来了。
这个提交还造成了下面你说的这个问题,很奇怪为什么当 stash pop 和 rebase 联用的时候,居然能让一个带冲突标记的文件顺利进入下一个 rebase 阶段。

3. #119 (files) 处类似的是什么?

总之现在我已经把造成问题的提交大卸八块,拆成了 9a455e0 (#119)15a2c7e (#119) 两个部分,希望这能减轻你审阅的负担(不过对前一个提交来说,因为 [] 条件式在脚本里用的太多了,所以还是会导致非常大的 diff)

@IceCodeNew
Copy link
Contributor Author

@teddysun

  1. libsodium 的源保持为 Github 比较好。

GitHub 上的 release 已经很久没发布更新了,参考官方文档,推荐的两种获取源代码的方式分别为以下途径(都是可以被认为稳定的代码):

  1. Download a tarball of libsodium, preferably the latest stable version
    https://doc.libsodium.org/installation#compilation-on-unix-like-systems

  2. We recommend using distribution tarballs over cloning the libsodium git repository, especially since tarballs do not require dependencies such as libtool and autotools.
    However, if cloning a git repository happens to be more convenient, the stable branch always contains the latest stable release of libsodium, plus minor patches that will be part of the next version, as well as critical security fixes while new packages including them are being prepared.
    https://doc.libsodium.org/installation#stable-branch

我研究过,GitHub 的 stable 分支和 https://download.libsodium.org/libsodium/releases/LATEST.tar.gz 地址下载下来的内容都包含同样的提交。所以从 download.libsodium.org 改为 GitHub 源没有问题,但是改成从 releases 里下载最后发布还是 2018 年的 assets 就不是很推荐了。


综上,我还是不能理解为何你希望 libsodium 的源 保持 为 GitHub,因为 libsodium 的源其实是 5 天前你才在 1e25c70 (#master) 提交里改成用 GitHub 源的,而且用的是比原来的源陈旧很多的源——有的场景下即使 GitHub releases 下的 assets 很旧了也没关系,但这里既然官方文档已经给出了建议,我觉得我们最好照着做。

这块我想我只有和你进一步讨论过后才能得到结论该怎么处理了。

@IceCodeNew
Copy link
Contributor Author

IceCodeNew commented Jul 6, 2020

2. 函数名func_decision_with_TRUE_or_FALSE_as_default_value不觉得太长了么?

是的,我同意你的看法。关于名字为什么这么长,其实是我为下一个提交提前预留的避免混淆的措施。
您看,我引入这个函数主要是为了缩减脚本的行数,原本的脚本里有太多几乎完全重复的逻辑了,非常有必要整合成一个函数。
而原本的脚本里其实还有相当多彼此相似的逻辑,虽然拿的也是用户输入,但期待的不是 yes/no 这样一个二元选择,而是 1/2/3/4 这样不同的选择支——我打算把这些逻辑也整合进函数里。但遇到点困难,就没有放在这次的 PR 里。
怕万一最后我发现引入的这一个函数不够,最后又追加了另一个关于用户输入的函数,那就有函数间彼此混淆的风险了。

也许我这个是杞人忧天吧,不知道您是否有些好的起名建议呢?我可以修改成您建议的函数名。
@teddysun


4. 安装时,选取的顺序不推荐更改,请保持原状。

已经 drop 掉相关修改。

@IceCodeNew
Copy link
Contributor Author

@teddysun ping...

@IceCodeNew
Copy link
Contributor Author

ping... @teddysun

@IceCodeNew
Copy link
Contributor Author

@teddysun ping...

@githuzz
Copy link

githuzz commented Feb 5, 2021

why there is cryptocurrency code showing up during the installation? is the code compromised? our server company already gave us a warning that cryptocurrency mining is not allowed. the crypto code shows up after below commands:
wget --no-check-certificate -O shadowsocks-all.sh https://raw.githubusercontent.com/teddysun/shadowsocks_install/master/shadowsocks-all.sh chmod +x shadowsocks-all.sh ./shadowsocks-all.sh 2>&1 | tee shadowsocks-all.log

@IceCodeNew
Copy link
Contributor Author

why there is cryptocurrency code showing up during the installation? is the code compromised? our server company already gave us a warning that cryptocurrency mining is not allowed. the crypto code shows up after below commands:
wget --no-check-certificate -O shadowsocks-all.sh https://raw.githubusercontent.com/teddysun/shadowsocks_install/master/shadowsocks-all.sh chmod +x shadowsocks-all.sh ./shadowsocks-all.sh 2>&1 | tee shadowsocks-all.log

You are supposed to ask questions by opening an issue, not reply in a PR, loon.

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.

3 participants