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

Add PHAR file to GitHub releases (PHIVE support) #531

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,17 @@ script:
after_success:
- bash <(curl -s https://codecov.io/bash)

before_deploy: make dist -j 4
before_deploy: make dist -j 4 && cp build/psysh/psysh dist/psysh.phar && cp build/psysh/psysh.asc dist/psysh.asc
Copy link
Owner

Choose a reason for hiding this comment

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

this should be done as part of the dist make target rather than inside .travis.yml

Copy link
Author

@filips123 filips123 Dec 1, 2019

Choose a reason for hiding this comment

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

As files already exist in build/psysh/psysh and build/psysh/psysh.asc, this (copying) is actually not needed, as I can directly release them to GitHub Releases.

Update: It is needed because psysh needs to be named psysh.phar and physh.asc needs to be named physh.phar.asc. I will move that to Makefile.

Copy link
Author

Choose a reason for hiding this comment

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

This is now fixed.

Copy link
Owner

Choose a reason for hiding this comment

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

Why does it need to be named psysh.phar? I’d really rather not.

Copy link
Author

Choose a reason for hiding this comment

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

Because PHIVE requires that.

Why you don't want this? In case of installation with PHIVE, .phar will be dropped so user when saving wouldn't notice it.

Copy link
Owner

Choose a reason for hiding this comment

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

For everyone who looks at the github release and doesn’t use phive. It means the most downloadable looking thing in the release is a .phar, which is bound to cause confusion. I’ve never loved this about phive. I feel like it forces suboptimal experiences for non-phive users, unless I want to maintain my own repo server.

Copy link
Author

Choose a reason for hiding this comment

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

But is this actually a problem? Because your website already shows installation with wget and composer. And if users need compat versions, they will probably already know that.

However, if you don't want that, you can set up custom repository server. I think that you can just use GitHub Pages for this with some CI script that automatically updates them at the new release. If you think this would be better, I can try to create new PR for it.


deploy:
provider: releases
api_key:
secure: LL8koDM1xDqzF9t0URHvmMPyWjojyd4PeZ7IW7XYgyvD6n1H6GYrVAeKCh5wfUKFbwHoa9s5AAn6pLzra00bODVkPTmUH+FSMWz9JKLw9ODAn8HvN7C+IooxmeClGHFZc0TfHfya8/D1E9C1iXtGGEoE/GqtaYq/z0C1DLpO0OU=
file_glob: true
file: dist/psysh-*.tar.gz
file:
- dist/psysh-*.tar.gz
- dist/psysh.phar
- dist/psysh.asc
skip_cleanup: true
on:
tags: true
Expand Down
22 changes: 20 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,31 @@ build/psysh-php54-compat: $(PSYSH_SRC) $(PSYSH_SRC_FILES)
build/%/psysh: vendor/bin/box build/%
vendor/bin/box compile --working-dir $(dir $@)

# Signatures

build/%/psysh.asc: build/%/psysh
bobthecow marked this conversation as resolved.
Show resolved Hide resolved
echo $$DECRYPT_KEY | gpg --batch --yes --passphrase-fd 0 .github/keys.asc.gpg
gpg --batch --yes --import .github/keys.asc

echo $$SIGN_KEY |\
gpg --passphrase-fd 0 \
-u $$KEY_ID \
--armor \
--detach-sig $(echo $@ | sed 's/.asc//') >\
$@

gpg --fingerprint --with-colons $$KEY_ID |\
grep "^fpr" |\
sed -n 's/^fpr:::::::::\([[:alnum:]]\+\):/\1/p' |\
xargs gpg --batch --delete-secret-keys
rm -rf .github/keys.asc

# Dist packages

dist/psysh-$(VERSION).tar.gz: build/psysh/psysh
dist/psysh-$(VERSION).tar.gz: build/psysh/psysh build/psysh/psysh.asc
bobthecow marked this conversation as resolved.
Show resolved Hide resolved
@mkdir -p $(@D)
tar -C $(dir $<) -czf $@ $(notdir $<)

dist/psysh-$(VERSION)-%.tar.gz: build/psysh-%/psysh
dist/psysh-$(VERSION)-%.tar.gz: build/psysh-%/psysh build/psysh-%/psysh.asc
bobthecow marked this conversation as resolved.
Show resolved Hide resolved
@mkdir -p $(@D)
tar -C $(dir $<) -czf $@ $(notdir $<)