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

added support for BSD based systems (specifically, OS X with homebrew getopt #19

Closed
wants to merge 1 commit into from

Conversation

wolph
Copy link

@wolph wolph commented Nov 8, 2019

On BSD/OS X the getopt command works differently so instead of parsing the arguments you get the original arguments back. On previous systems I've simply changed my PATH to fix it but I thought a better fix would be to check for it in the code :)

@NicoHood
Copy link
Owner

Hey @wolph It seems I completely missed this PR. Thanks for taking the time to share your patch!

I recently pushed some more bugfixes and changes (changelog will be updated soon, so you can follow better). It would be nice, if you can give it a test on BSD. I will try to integrate your changes from this PR as well.

@NicoHood
Copy link
Owner

I've adopted the fix here, can you please verify it is working?
ab76a7e

@NicoHood NicoHood closed this Jan 26, 2021
@wolph
Copy link
Author

wolph commented Jan 30, 2021

It mostly appears to be working. But I am getting this error (although I think it's unrelated to this change):

==> ERROR: Github API message: 'Problems parsing JSON' Check your token configuration: https://github.com/settings/tokens

No clue what's causing it. I already tried to update my token but no dice.

@NicoHood
Copy link
Owner

Could you please post the terminal output? You can also try with --debug, but please remove any sensible information first (like the token).

@NicoHood
Copy link
Owner

NicoHood commented Jan 30, 2021

I've tried to fix it via ae0a3ff
You can test the dev branch. I assume that one of the parameters you were using were not escaped properly. Did you use a custom name, tag or message?

@wolph
Copy link
Author

wolph commented Jan 31, 2021

Yes, that was probably it. The new release works perfect! Thank you for the fix :)

@NicoHood
Copy link
Owner

I've published a new release with the fix.

Could you maybe help me write some installing instructions for those systems? Mainly the command to install the dependencies and their correct name would be important.

I also want to add a feature that checks the key expire date. Now I found that date is different for gnu vs Mac/BSD Systems. You'd need to use gdate (gnu date) there instead, similar to getopt. I might get back to you if I need further help or testing with this.

@wolph
Copy link
Author

wolph commented Feb 1, 2021

Sure, it's rather simple I believe :)

  1. Install Homebrew: https://brew.sh/
  2. Install gnu-getopt: brew install gnu-getopt
  3. Install coreutils for gnu date: brew install coreutils

And that's it :)

Optionally (if people don't have it yet)

  1. brew install git
  2. brew install curl

Or an all-in-one command: brew install curl git gnu-getopt coreutils

@wolph
Copy link
Author

wolph commented Feb 3, 2021

@NicoHood still not perfectly fixed unfortunately. The issue is with quotes in MESSAGE that completely breaks.

@wolph
Copy link
Author

wolph commented Feb 3, 2021

Actually... this seems to fix it for me:

SAFE_MESSAGE=${MESSAGE//$'\n'/'\n'}
SAFE_MESSAGE=${SAFE_MESSAGE//\"/\\\"/}
API_JSON="$(printf '{"tag_name": "%s","target_commitish": "%s","name": "%s","body": "%s","draft": false,"prerelease": %s}' \
           "${TAG}" "${BRANCH}" "${TAG}" "$SAFE_MESSAGE" "${PRERELEASE}")"

@NicoHood
Copy link
Owner

NicoHood commented Feb 3, 2021

Can you name me a sample message, that does not work for you? I've replaced printf with jq, it should escape everything. I am wondering why this still does not work properly.

@wolph
Copy link
Author

wolph commented Feb 3, 2021

I've recently switched to MESSAGE=$(git log -1)

commit 638f67af428413ef6b2a45633b6e4f034b11f272
Merge: ad33bc1 061837e
Author: Rick van Hattem <Wolph@wol.ph>
Date:   Wed Feb 3 02:10:59 2021 +0100

    Merge tag 'v2.2.1' into develop
    
    removed debug statement v2.2.1
    
    # gpg: Signature made Wed Feb  3 02:10:59 2021 CET
    # gpg:                using RSA key 149325FD15904E9C4EB89E95E81444E9CE1F695D
    # gpg: Good signature from Rick van Hattem <wolph@wol.ph> [ultimate]
    # gpg:                 aka [jpeg image of size 9662] [ultimate]

@NicoHood
Copy link
Owner

NicoHood commented Feb 3, 2021

Why would you do this? This message also gets attached to the tag and the github message.

Did you see, that I've added automatically parsing of a changelog file in 1.4.x? I am currently working a lot on this project, including a website etc, so everything will be (hopefully) more clear soon. Maybe that feature could be useful for you too.

@wolph
Copy link
Author

wolph commented Feb 3, 2021

Because Github doesn't show the tag commit message in the list of releases: https://github.com/WoLpH/python-utils/releases
That output is pretty useless.

So changed it to look like this: https://github.com/WoLpH/portalocker/releases
I still need to add the mesage to the title it seems, but it's already far more useful.

I didn't know about the changelog parsing, but I'm not using a separate changelog anyhow. The tag commit messages are my changelog.

@NicoHood
Copy link
Owner

NicoHood commented Feb 3, 2021

I am not sure if I understand correct.

Did you try the -m option? `gpgit 1.0.0 -m "This is my tag message. Here I can add a changelog for example."

Also looking at your post above you said you are using MESSAGE=$(git log -1). Did you edit the code directly? If yes, the issue is possibly related to missing quotes: MESSAGE="$(git log -1)", but even better is to not modify the script and use gpgit 1.0.0 -m "$(git log -1)".

I am still wondering what useful information git log -1 contains. This sounds like nothing important, but I might be wrong. Keep A Changelog has some good arguments, why a dedicated changelog makes sense.

Did that help or am I still completely missing something?

Offtopic: Is there any reason why you are tagging releases with "v" v1.0.0 prefix? Cause that is currently not supported for parsing changelogs. I can think of some reasons, but want to hear your opinion as well ;-)

@wolph
Copy link
Author

wolph commented Feb 3, 2021

I am not sure if I understand correct.

Did you try the -m option? `gpgit 1.0.0 -m "This is my tag message. Here I can add a changelog for example."

Also looking at your post above you said you are using MESSAGE=$(git log -1). Did you edit the code directly? If yes, the issue is possibly related to missing quotes: MESSAGE="$(git log -1)", but even better is to not modify the script and use gpgit 1.0.0 -m "$(git log -1)".

Let me clarify a bit :)

I use git-flow release to create releases and in the post-flow-release-finish hook I have the following to tell gpgit to create a release:

MESSAGE=$(git log -1)

# Implement your script here.
gpgit $VERSION -m "$MESSAGE"

Perhaps I should replace it with MESSAGE=$(git tag -n9 $VERSION) instead. For now it was just a quick way to get something useful in the github releases.

I am still wondering what useful information git log -1 contains. This sounds like nothing important, but I might be wrong. Keep A Changelog has some good arguments, why a dedicated changelog makes sense.

My git tags are the changelog in this case. It contains all of the relevant changes made in the releases.

While I agree that dedicated changelogs are better... I know I'll forget and having an out of date changelog is worse than not having a dedicated one. Besides slightly different formatting it's effectively the same.

Offtopic: Is there any reason why you are tagging releases with "v" v1.0.0 prefix? Cause that is currently not supported for parsing changelogs. I can think of some reasons, but want to hear your opinion as well ;-)

It's mostly legacy now to be honest. Originally it was because some software automatically tries to convert it into a number which fails because there are 2 dots.

@wolph
Copy link
Author

wolph commented Feb 3, 2021

I think I do have a little feature request. It would be nice to be able to set the release title as well. That seems to be hardcoded to the tag right now.

@NicoHood
Copy link
Owner

NicoHood commented Feb 3, 2021

Thanks for the feedback. I've opened separated bugs for this.

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.

None yet

2 participants