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

pdfcpu version panics with runtime error #751

Closed
2 tasks done
afh opened this issue Dec 10, 2023 · 11 comments
Closed
2 tasks done

pdfcpu version panics with runtime error #751

afh opened this issue Dec 10, 2023 · 11 comments
Assignees

Comments

@afh
Copy link
Contributor

afh commented Dec 10, 2023

When trying to update the pdfcpu package in nixpkgs to 0.6.0 the installCheck fails. The issue is reproducible when deactivating the installCheck and running pdfcpu version manually:

% pdfcpu version
pdfcpu: v0.6.0 dev
panic: runtime error: slice bounds out of range [:8] with length 1

goroutine 1 [running]:
main.printVersion(0x140001e52b0)
        github.com/pdfcpu/pdfcpu/cmd/pdfcpu/process.go:143 +0x24c
main.commandMap.process(0x140001c5ec8?, {0x16d413117, 0x7}, {0x0, 0x0})
        github.com/pdfcpu/pdfcpu/cmd/pdfcpu/cmd.go:143 +0x2e4
main.main()
        github.com/pdfcpu/pdfcpu/cmd/pdfcpu/main.go:56 +0xac

Thank you for submitting a possible bug!

Please ensure the following:

  • Your issue is based on the latest commit release
  • State your OS and OS version
% sw_vers
ProductName:            macOS
ProductVersion:         14.1.2
BuildVersion:           23B92
  • N/A When reporting a problem with a specific PDF input file please avoid stating the organization responsible for the PDFWriter - just refer to the PDFWriter
@hhrutter
Copy link
Collaborator

hhrutter commented Dec 10, 2023

Please pull the release again and let me know if that works for you.
I had to push force the latest commit around noon GMT today.

Although I don't think this is the cause of this, the output of pdfcpu version has changed and should look something like:

pdfcpu: v0.6.0 dev
commit: e3358c4c (2023-12-10T09:32:30Z)
base  : go1.21.4
config: /Users/horstrutter/Library/Application Support/pdfcpu/config.yml

@afh
Copy link
Contributor Author

afh commented Dec 10, 2023

Thanks for the info, @hhrutter, much appreciated! When building locally from a clone of this repo pdfcpu version works as expected. I'll dig deeper into the issues when building pdfcpu 0.6.0 via nixpkgs and will report back.

afh added a commit to afh/pdfcpu that referenced this issue Dec 10, 2023
@afh
Copy link
Contributor Author

afh commented Dec 10, 2023

@hhrutter looking at the implementation of printVersion it seems the code expects to always be build from a git repo clone and not from a distribution archive.

If I'm not mistaken then debug.ReadBuildInfo() could return a BuildInfo that does not contain any of the vcs related settings like revision. This could lead to commit not being initialized properly and commit[:8] to panic as commit only contains the string "?", which is too short to be sliced to 8 characters.

The issue can easily be reproduced by temporarily moving the .git directory, e.g. mv .git _git.

For a possible fix please see #752 🙂

@hhrutter
Copy link
Collaborator

Yes I am aware of that and that safeguard does not hurt.
Still, I am curious, were you able to reproduce a situation where this is the case?
I wonder if pulling in the middle of me push forcing was the reason for this inconsistency.

@afh
Copy link
Contributor Author

afh commented Dec 10, 2023

I'm uncertain whether I understood your question correctly, so I'm going to answer what I think you asked :)
Yes, I'm able to reproduce a situation where BuildInfo does not contain any of the vcs related settings:

  • Either temporarily move the .git folder in your repo clone
cd $PATH_TO_REPO/pdfcpu
mv .git _git
(cd cmd/pdfcpu && go build && ./pdfcpu version)
mv _git .git
  • or build from a distribution archive
curl -#L https://github.com/pdfcpu/pdfcpu/archive/refs/tags/v0.6.0.tar.gz | tar zxf -
(cd pdfcpu-0.6.0/cmd/pdfcpu && go build && ./pdfcpu version)

@hhrutter
Copy link
Collaborator

Why would anybody remove the .git repo from the sources before building?
Is that the procedure at NixOS?

Just ignoring expected parameters during pdfcpu version is not a solution.
The real issue here is pdfcpu version needs to produce the expected output - otherwise the build is considered a failed one, so even good that you are checking pdfcpu version.

So whatever needs to be observed during building has to be part of the install instructions.

@afh
Copy link
Contributor Author

afh commented Dec 10, 2023

It's not that NixOS is actively removing the .git directory before building, but prefers to download distribution archives, which do not contain a .git directory.

NixOS provides options, e.g. leaveDotGit or forceFetchGit, to fetch sources including the .git directory, for some odd reasons the reported issue persists with either of those options 🙁 I'm looking further into it…

@afh
Copy link
Contributor Author

afh commented Dec 10, 2023

Thank you for your comments and questions, @hhrutter, they kept me on my toes and helped me find a way to update pdfcpu in NixOS without requiring changes in this repo (see NixOS/nixpkgs#273438).

I can see how the fix or rather workaround proposed in #752 is less than ideal and maybe a better approach would be to not omit the "commit" line, but rather display unknown in when vcs information isn't available.

From my point of view there is value in addressing the panic caused by the possibility that the commit variable is initialized with "?" and printVersion is trying to create slice that extends beyond the initial length of commit.

From my end this issue and #752 can be considered close, yet I'm happy to help improving pdfcpu in this regard if you see value in it and I can be helpful 🙂

@doronbehar
Copy link

Why would anybody remove the .git repo from the sources before building?
Is that the procedure at NixOS?

NixOS provides options, e.g. leaveDotGit or forceFetchGit, to fetch sources including the .git directory, for some odd reasons the reported issue persists with either of those options 🙁 I'm looking further into it…

Nix requires sources downloaded from the internet to have the same output content bit by bit, i.e deterministic. Apparently, for complicated reasons, Git clones are not deterministic, and we investigate this issue in NixOS/nixpkgs#8567 .

afh added a commit to afh/pdfcpu that referenced this issue Dec 12, 2023
@jacobhokanson
Copy link

@hhrutter the same panic occurs if you install pdfcpu directly as in the readme
image

@hhrutter
Copy link
Collaborator

👍🏻 Thanks for pointing that out. Will check..

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

No branches or pull requests

4 participants