Skip to content

Portindex: Add extended mode#200

Merged
mojca merged 1 commit into
macports:masterfrom
arjunsalyan:extended-portindex
Aug 2, 2020
Merged

Portindex: Add extended mode#200
mojca merged 1 commit into
macports:masterfrom
arjunsalyan:extended-portindex

Conversation

@arjunsalyan

@arjunsalyan arjunsalyan commented Jul 26, 2020

Copy link
Copy Markdown
Member

This commit adds a flag -x (for extended mode) which will allow to include more details in the Portindex, like notes, variant description etc.

Why is this needed?
We are trying to add variant description and port notes to the ports webapp. By faking the platform (portindex -p), we generate portindex for the latest MacOS and store it in the database. But the server and docker both run Linux, and hence the local ports tree is different from that generated by portindex -p. This renders port variants and port notes commands displaying incomplete/inconsistent results.

In addition to this, an extended portindex would also simplify the process of updating the database. We would be able to follow a single route, instead of three (which is being done right now).

arjunsalyan added a commit to arjunsalyan/macports-contrib that referenced this pull request Jul 27, 2020
arjunsalyan added a commit to arjunsalyan/macports-contrib that referenced this pull request Jul 27, 2020
@arjunsalyan arjunsalyan requested a review from jmroot July 27, 2020 09:10
@mojca

mojca commented Jul 31, 2020

Copy link
Copy Markdown
Member

@neverpanic, @jmroot: any objections?

@ryandesign

Copy link
Copy Markdown
Contributor

Is the intention for this to be a feature you would use locally on that linux server, or that the use of this flag would be incorporated on the server that runs mprsyncup?

@mojca

mojca commented Jul 31, 2020

Copy link
Copy Markdown
Member

We would definitely like to use it locally on the server running the web app since we might potentially want to run portindex after each commit at some point, or at least more frequently / in more predictable time intervals than polling the server all the time, whether or not anything has changed or not.

Regarding the PortIndex.json on the server:

  • In theory I would prefer to publish the extended .json file. The only annoyance is that it's a tiny bit annoying to have to generate the PortIndex twice (once in its original form for upload, and once just a temporary version to generate the extended .json out of it).
  • Since we won't be using those data directly when the new app gets deployed, I don't care that much whether or not we publish the extended info in the .json. For all I know that file is currently used by repology, no clue if anyone else uses it as well. For most third parties it will likely be much easier to consult the app's API directly rather than fetching the full .json file.
  • We can discuss what to run inside the mprsyncup job later at any given time. But this particular change would be of extreme help in simplifying the deployment of the app right now. If possible, I would like to see this merged by tomorrow, so that we can start test deployment on the new server.

@ryandesign

Copy link
Copy Markdown
Contributor

Anything done locally on that server is fine; you can of course apply this change to your local copy of MacPorts already and use it there.

If you can run it locally on that server, I'm not sure why you need it to be generated on a different server and distributed to you over the network. As you say there would be additional complications in needing to generate a separate extended copy of the portindex to then be converted to json for the web app to use.

I was concerned that this would increase the size of the portindex greatly but it's not that bad really:

$ ls -lh PortIndex* extended/PortIndex*
-rw-r--r--  1 root      wheel    14M Jul 31 13:08 PortIndex
-rw-r--r--  1 rschmidt  staff    23M Jul 31 13:28 PortIndex.json
-rw-r--r--  1 rschmidt  staff   545K Jul 31 13:08 PortIndex.quick
-rw-r--r--  1 root      wheel    16M Jul 31 13:08 extended/PortIndex
-rw-r--r--  1 rschmidt  staff    28M Jul 31 13:28 extended/PortIndex.json
-rw-r--r--  1 root      staff   548K Jul 31 13:26 extended/PortIndex.quick

I don't like the introduction of a flag called -extended. If we were to introduce a flag whose name is more than one letter, then it should be preceded by 2 dashes, not 1 (--extended). But the way that we handle flags in port is already weird, with single-letter single-dash flags having a different meaning and context from multi-letter two-dash flags. I would not like for portindex to introduce a different weird flag handling method. Since all the existing flags in portindex are single-letter single-dash, maybe that tradition should be continued and the new flag would be -x.

The new flag should be included in the portindex help output.

@arjunsalyan

Copy link
Copy Markdown
Member Author

If you can run it locally on that server, I'm not sure why you need it to be generated on a different server and distributed to you over the network. As you say there would be additional complications in needing to generate a separate extended copy of the portindex to then be converted to json for the web app to use.

Once the new changes are deployed, webapp would not need the JSON from the rsync server. Repology uses the normal version of the portindex.json.

I don't like the introduction of a flag called -extended. If we were to introduce a flag whose name is more than one letter, then it should be preceded by 2 dashes, not 1 (--extended). But the way that we handle flags in port is already weird, with single-letter single-dash flags having a different meaning and context from multi-letter two-dash flags. I would not like for portindex to introduce a different weird flag handling method. Since all the existing flags in portindex are single-letter single-dash, maybe that tradition should be continued and the new flag would be -x.

The new flag should be included in the portindex help output.

Thank you. portindex -x seems better, unless others have any objections/ different suggestions.

@mojca

mojca commented Aug 1, 2020

Copy link
Copy Markdown
Member

If you can run it locally on that server, I'm not sure why you need it to be generated on a different server and distributed to you over the network.

As Arjun already said, we don't really need the extended portindex generated on the server (all I was saying is that if we are already serving the json, we could just as well serve a slightly better json).

The reason why we would like to see this merged ASAP is not for the sake of being able to generate the extended PortIndex remotely, but for the sake of avoiding the need to fork and patch MacPorts sources to run the web app.

This  commit adds a flag "-x" (for extended mode) which will allow to include more details in the Portindex, like notes, variant description etc.
@mojca mojca merged commit 0b8b970 into macports:master Aug 2, 2020
arjunsalyan added a commit to macports/macports-contrib that referenced this pull request Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants