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

Reduce dependencies #271

Closed
wants to merge 5 commits into from
Closed

Conversation

JingMatrix
Copy link
Contributor

These commits will use system-wise libraries for mupdf, sqlite3 and utf8.
I tested it on my Ubuntu machine, not able to test for windows.

In the following commits, we will gradually use portable or
system-wise libraries for sqlite3, utf8 and synctex.
Use latestest version 1.27 released in 2017 for synctex_parser.h.
Forward-search now supports column number parameter.
@JingMatrix
Copy link
Contributor Author

I removed git-submodules, so the above build release won't pass.
Please tell on Windows or Mac to see if it works @ahrm .

@JingMatrix
Copy link
Contributor Author

Or you can keep this pull-request open, (or you can create a new branch), to simplify building process for linux users.

@ahrm
Copy link
Owner

ahrm commented Jun 12, 2022

I am hesitant to accept changes that modify dependencies because getting them working in multiple operating systems was a nightmare.

@ahrm ahrm closed this Jun 12, 2022
@JingMatrix
Copy link
Contributor Author

If I guess correctly, most dependencies problem comes from Windows instead of Mac, is that right?
Personally, I would suggest using CMake buiding system to improve this problem.

@ahrm
Copy link
Owner

ahrm commented Jun 12, 2022

Actually windows was quite easy. The main problem is various linux distributions and minor package difference between them.

@JingMatrix
Copy link
Contributor Author

Could you give some expamles for your comments on various linux distributions? For exmaple, are there some related issues in the list?
Because I am really not convinced of this.

@ahrm
Copy link
Owner

ahrm commented Jun 12, 2022

For example ubuntu 18.04 LTS has an outdated version of mupdf that doesn't work with sioyek so we have to compile mupdf from source but then sometimes the libharfbuzz package from the system clashes with the libharfbuzz in mupdf source code and causes a hard crash, but if we don't install libharfbuzz there are problems for users who want to use their system's mupdf (there are multiple github issues about that). Most distributions don't need to explicitly install gumbo-parser because it is included in the mupdf package but some do. The name of some packages are different in different distributions, etc. etc.

I am hesitant to accept changes that change dependencies unless you explicitly test in on following systems:

  • Windows 10+
  • MacOS latest version
  • Ubuntu 18.04 LTS and 22.04 LTS
  • Archlinux

But your change for column handling in synctex is good, why don't you make a pull request without changing the dependencies?

@JingMatrix
Copy link
Contributor Author

Well, my personal opinion is that we should use CMake build system to explicitly require the dependencies versions.
Providing (common) dependencies, such as mupdf, is the task of users instead of maintainers.
Usually, I believed that we need a sub-module only when we have a modified fork for our purpose.
This is just my personal ideas from experience, you may think differently.
For long-term maintenance, I strongly recommand so.

I am on Ubuntu 22.10, and it passed the compilation and works smoothly.
I don't use Windows 10, no way for me to test it.

With a copy of synctex in the repo for me is not a good idea.
I made this pull-request because the old version of synctex isn't working on my computer. So I turned to the latest version.

I will cherry pick the synctex and send a pull-request for it.

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