Merge and update `about-ql-packs` with `about-codeql-packs` #10105

aeisenberg · 2022-08-18T22:29:16Z

This is the first of a series of commits around updating packaging docs.

about-ql-packs.rst is outdated. All relevant information has been
moved to about-codeql-packs.rst.

Also, this goes through all of the other main documentation pages to update them for packaging.

This is the first of a series of commits around updating packaging docs. `about-ql-packs.rst` is outdated. All relevant information has been moved to about-codeql-packs.rst`.

In this commit: - Replace _QL pack_ with _CodeQL pack_ - Replace `about-ql-pack` references with `about-codeql-packs` - Replace examples using `libraryPathDependencies with `dependencies` - Update some examples to specify the optional `version` field - Update description of query specifiers to note that a path within a pack is valid.

aeisenberg · 2022-08-19T20:31:48Z

It would be great to get lots of different eyes on this. I would like to have a technical review first before getting more of a copy-editing review from someone on the docs team.

aeisenberg · 2022-08-19T20:32:10Z

@jf205 Should we be removing the beta comment for all packaging features?

jf205 · 2022-08-22T13:59:58Z

Should we be removing the beta comment for all packaging features?

Thanks for asking. CodeQL packaging is still in beta, so let's leave the comments in place.

henrymercer

A bunch of suggestions, several minor/optional:

docs/codeql/codeql-cli/about-codeql-packs.rst

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

docs/codeql/codeql-cli/creating-and-working-with-codeql-packs.rst

docs/codeql/codeql-cli/creating-codeql-query-suites.rst

docs/codeql/codeql-cli/getting-started-with-the-codeql-cli.rst

henrymercer

A few more comments, otherwise LGTM.

docs/codeql/reusables/test-qlpack.rst

docs/codeql/codeql-cli/about-codeql-packs.rst

Co-authored-by: Henry Mercer <henrymercer@github.com>

jf205

Thanks so much for making a start on this @aeisenberg. I've passed through once and have added a round of comments.

I'd like to look again with fresh eyes soon. It would be good if we could trigger the Docs - Generate Sphinx CI check -- that checks the RST formatting and renders the articles so that we can see what the changes will look like on the docs site.

jf205 · 2022-08-23T15:55:09Z

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

-   You shouldn't specify the root of a :doc:`QL pack
-   <about-ql-packs>` when executing ``database analyze``
+   You shouldn't specify the root of a :doc:`CodeQL pack
+   <about-codeql-packs>` when executing ``database analyze``
   as it contains some special queries that aren't designed to be used with
   the command. Rather, to run a wide range of useful queries, run one of the
   LGTM.com query suites.


Can we change this to 'code scanning query suites' while we are here?

Right...I haven't done a search for LGTM references. Looks like there are quite a few. Updating them is outside the scope of this change, but I will change this lgtm reference.

We should schedule some work to update other lgtm references.

Thanks. We'll address the other LGTM references elsewhere. I think it makes sense to change this while we are so close to it!

jf205 · 2022-08-23T15:57:43Z

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

@@ -239,18 +240,23 @@ recursively, so any queries contained in subfolders will also be executed.

   Important

-   You shouldn't specify the root of a :doc:`QL pack
-   <about-ql-packs>` when executing ``database analyze``
+   You shouldn't specify the root of a :doc:`CodeQL pack


Perhaps we should make clear that this applies to the standard CodeQL packs? Custom CodeQL packs might not contain any special queries.

It also looks like the order is a bit off here too. Do you think "Running all queries in a directory" should appear before "Integrating a CodeQL pack into a code scanning workflow in GitHub"?

Yes. I'll change the order.

Custom packs might, or might not contain custom special queries. If you run the pack, it's generally safer since you don't know unless you inspect the pack. It's always safer to run the pack instead of the directory since there's no danger of running special queries.

I'll change it to say something like that.

Actually, I take that back, this paragraph really is focused on core packs.

jf205 · 2022-08-23T20:00:08Z

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst


   codeql database analyze <python-database> ../ql/python/ql/src/Functions/ --format=sarif-latest --output=python-analysis/python-results.sarif

-A SARIF results file is generated. Specifying ``--format=sarif-latest`` ensures
+If you do not have the CodeQL repository checked out, you can execute the same queries by specifying the query pack name and the path to the queries:


Suggested change

If you do not have the CodeQL repository checked out, you can execute the same queries by specifying the query pack name and the path to the queries:

If you do not have the CodeQL repository checked out, you can execute the same queries by specifying the query pack name and the path to the queries::

jf205 · 2022-08-23T20:01:53Z

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

+
+Use the ``--download`` flag to download the query pack if it isn't yet available locally.
+
+After evaluating, a SARIF results file is generated. Specifying ``--format=sarif-latest`` ensures


Suggested change

After evaluating, a SARIF results file is generated. Specifying ``--format=sarif-latest`` ensures

When the analysis has finished, a SARIF results file is generated. Specifying ``--format=sarif-latest`` ensures

jf205 · 2022-08-23T20:02:54Z

docs/codeql/codeql-cli/creating-and-working-with-codeql-packs.rst

@@ -34,24 +34,26 @@ You must specify:

 The ``codeql pack init`` command creates the directory structure and configuration files for a CodeQL pack. By default, the command creates a query pack. If you want to create a library pack, you must edit the ``qlpack.yml`` file to explicitly declare the file as a library pack by including the ``library:true`` property.

-Modifying an existing QL pack to create a CodeQL pack
-----------------------------------------------------
+Modifying an existing legacy CodeQL pack to create a CodeQL pack


Perhaps we should still refer to the old-style packs as "QL packs" to avoid confusion?

Suggested change

Modifying an existing legacy CodeQL pack to create a CodeQL pack

Modifying an existing legacy QL pack to create a CodeQL pack

docs/codeql/codeql-cli/about-codeql-packs.rst

jf205 · 2022-08-23T20:31:13Z

docs/codeql/codeql-cli/about-codeql-packs.rst

+.. _standard-ql-packs:
+
+Examples of CodeQL packs in the CodeQL repository
+---------------------------------------------~~~~


Suggested change

---------------------------------------------~~~~

--------------------------------------------------

jf205 · 2022-08-23T20:34:16Z

docs/codeql/codeql-cli/creating-codeql-query-suites.rst

+     version: ^x.y.z
+
+  The ``version`` field is optional and specifies a range of compatible versions of this CodeQL pack.
+  If the version is excluded, then the most recent version of the pack is used.


Suggested change

If the version is excluded, then the most recent version of the pack is used.

If you don't specify a version, then the most recent version of the pack is used.

jf205 · 2022-08-23T20:34:37Z

docs/codeql/codeql-cli/creating-codeql-query-suites.rst


  The default suite of a query pack includes a recommended set of queries
  inside of that query pack. Not all query packs have a default suite. If the given query pack does not
  define a default suite, the `qlpack` instruction will resolve to all of the queries within the pack.

+  The ``version`` field is optional and specifies a range of compatible versions of this CodeQL pack.
+  If the version is excluded, then the most recent version of the pack is used.


Suggested change

If the version is excluded, then the most recent version of the pack is used.

If you don't specify a version, then the most recent version of the pack is used.

jf205 · 2022-08-23T20:39:11Z

docs/codeql/codeql-cli/creating-codeql-query-suites.rst

+      version: ^x.y.z
+
+  The ``version`` field is optional and specifies a range of compatible versions of this CodeQL pack.
+  If the version is excluded, then the most recent version of the pack is used.


Suggested change

If the version is excluded, then the most recent version of the pack is used.

If you don't specify a version, then the most recent version of the pack is used.

aeisenberg · 2022-08-24T00:09:25Z

@jf205 here is the job that is building the docs. https://github.com/github/semmle-code/runs/7985186319?check_suite_focus=true When it is complete, it should contain a url.

aeisenberg · 2022-08-25T18:37:28Z

I addressed your changes two days ago, but I pushed them to the wrong branch. Fixing that now.

dbartol

A few suggestions around wording, plus some details on QL tests and lock files.

docs/codeql/codeql-cli/about-codeql-packs.rst

Co-authored-by: Dave Bartolomeo <dbartol@github.com>

dbartol · 2022-08-29T21:27:57Z

docs/codeql/codeql-cli/about-codeql-packs.rst

 ..
  TODO: Add a link to the CodeQL CLI documentation for query resolution, specifically in regards to resolving from source

 The ``codeql/cpp-all`` dependency is locked to version 0.1.4. The ``my-user/my-lib`` dependency is locked to version 0.2.4. The ``my-user/transitive-dependency``, which is a transitive dependency and is not specified in the ``qlpack.yml`` file, is locked to version 1.2.4. The ``other-dependency/from-source`` is absent from the lock file since it is resolved from source. This dependency must be available in the same CodeQL workspace as the pack.

+In most cases, the ``codeql-pack.lock.yml`` file is only relevant for query packs since library packs are non-executable and usually do not need their transitive dependencies to be fixed. The exception to this is for library packs that contain tests. In this case, the ``codeql-pack.lock.yml`` file is used to ensure that the tests are always run with the same versions of dependencies to avoid spurious failures when there are mismatched dependencies.


I'm not 100% sure, but I believe that lock files for library packs may also influence the VS Code QL language server, especially if the repo has only library packs. So, it's not just for tests. That said, I think this paragraph that just mentions tests is fine.

Oh...I didn't know that. Makes sense, though.

On line 26 we say that lock files are relevant for Query packs. It's possible that we should make a minor update to that line since it sounds as if you may get a lock file with all pack types?

I made a small change.

dbartol

LGTM, if the other reviewers are happy with how you've addressed their comments as well.

docs/codeql/codeql-cli/about-codeql-packs.rst

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

jf205 · 2022-09-01T09:17:21Z

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

+Use the ``--download`` flag to download the query pack if it isn't yet available locally.
+
+.. _run-query-pack:
+
 Running a CodeQL pack


I'm still not sure the order is right here and we have a bit of repetition, but maybe that's my fault for making a confusing suggestion in an earlier review. Let's not fix it in this PR though. I'll open an issue with my thoughts.

jf205 · 2022-09-01T09:20:20Z

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

+If you do not have the CodeQL repository checked out, you can execute the same queries by specifying the query pack name and the path to the queries::
+
+   codeql database analyze --download <python-database> codeql/python-queries:Functions/ --format=sarif-latest --output=python-analysis/python-results.sarif
+
+Use the ``--download`` flag to download the query pack if it isn't yet available locally.
+


This is almost a repetition of what is in the section on CodeQL packs above. Can we leave it out for now and address the structure of this whole section in a separate issue?

I'll remove that. Really, we should have the pack variant of the command coming first and the filesystem variant of the command second (if at all).

felicitymay

Thanks for all these updates to the docs 💖

I haven't checked every single link in the test output, but generally this looks good and I've just added a few small comments.

docs/codeql/codeql-cli/about-codeql-packs.rst

docs/codeql/codeql-cli/query-reference-files.rst

docs/codeql/codeql-cli/using-custom-queries-with-the-codeql-cli.rst

docs/codeql/reusables/test-qlpack.rst

docs/codeql/codeql-cli/about-codeql-packs.rst

aeisenberg · 2022-09-02T19:57:54Z

docs/codeql/codeql-cli/about-codeql-packs.rst

 ..
  TODO: Add a link to the CodeQL CLI documentation for query resolution, specifically in regards to resolving from source

 The ``codeql/cpp-all`` dependency is locked to version 0.1.4. The ``my-user/my-lib`` dependency is locked to version 0.2.4. The ``my-user/transitive-dependency``, which is a transitive dependency and is not specified in the ``qlpack.yml`` file, is locked to version 1.2.4. The ``other-dependency/from-source`` is absent from the lock file since it is resolved from source. This dependency must be available in the same CodeQL workspace as the pack.

+In most cases, the ``codeql-pack.lock.yml`` file is only relevant for query packs since library packs are non-executable and usually do not need their transitive dependencies to be fixed. The exception to this is for library packs that contain tests. In this case, the ``codeql-pack.lock.yml`` file is used to ensure that the tests are always run with the same versions of dependencies to avoid spurious failures when there are mismatched dependencies.


Oh...I didn't know that. Makes sense, though.

docs/codeql/codeql-cli/about-codeql-packs.rst

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

aeisenberg · 2022-09-02T20:04:42Z

docs/codeql/codeql-cli/analyzing-databases-with-the-codeql-cli.rst

+If you do not have the CodeQL repository checked out, you can execute the same queries by specifying the query pack name and the path to the queries::
+
+   codeql database analyze --download <python-database> codeql/python-queries:Functions/ --format=sarif-latest --output=python-analysis/python-results.sarif
+
+Use the ``--download`` flag to download the query pack if it isn't yet available locally.
+


I'll remove that. Really, we should have the pack variant of the command coming first and the filesystem variant of the command second (if at all).

Co-authored-by: Felicity Chapman <felicitymay@github.com> Co-authored-by: James Fletcher <42464962+jf205@users.noreply.github.com>

felicitymay

Thanks for addressing my comments and for answering my questions. This looks pretty much there. I responded to a couple of conversations, but once those are resolved, this looks ready to merge.

aeisenberg · 2022-09-06T21:01:08Z

Addressed your comments in a few more commits.

felicitymay

Happy for you to merge this if other reviewers are happy 🚀

aeisenberg · 2022-09-07T15:05:03Z

@jf205 has some suggestions for a followup PR, but it looks like others are happy. Thanks for the reviews.

This moves the following three PRs to the 3.7 branch: - #10182 - #10146 - #10105

aeisenberg requested a review from dbartol August 18, 2022 22:29

github-actions bot added the documentation label Aug 18, 2022

Merge and update about-ql-packs with about-codeql-packs

d737b57

This is the first of a series of commits around updating packaging docs. `about-ql-packs.rst` is outdated. All relevant information has been moved to about-codeql-packs.rst`.

aeisenberg force-pushed the aeisenberg/about-codeql-packs branch from 77c0fb1 to d737b57 Compare August 18, 2022 22:31

aeisenberg marked this pull request as draft August 19, 2022 15:52

aeisenberg force-pushed the aeisenberg/about-codeql-packs branch from bc3d2b2 to 3890907 Compare August 19, 2022 20:14

aeisenberg marked this pull request as ready for review August 19, 2022 20:30

aeisenberg requested a review from a team August 19, 2022 20:30

henrymercer reviewed Aug 22, 2022

View reviewed changes

Address comments from PR

d892304

henrymercer previously approved these changes Aug 23, 2022

View reviewed changes

Apply suggestions from code review

a40b7ef

Co-authored-by: Henry Mercer <henrymercer@github.com>

aeisenberg dismissed henrymercer’s stale review via a40b7ef August 23, 2022 15:57

jf205 reviewed Aug 23, 2022

View reviewed changes

Apply suggestions from code review

5fe06ab

Fix typos in docs

2ad0b28

dbartol reviewed Aug 29, 2022

View reviewed changes

aeisenberg and others added 2 commits August 29, 2022 14:00

Apply suggestions from code review

f737e07

Co-authored-by: Dave Bartolomeo <dbartol@github.com>

Add paragraph about lock files in library packs with tests

e14b803

dbartol reviewed Aug 29, 2022

View reviewed changes

dbartol previously approved these changes Aug 29, 2022

View reviewed changes

aeisenberg added the ready-for-doc-review This PR requires and is ready for review from the GitHub docs team. label Aug 30, 2022

jf205 reviewed Sep 1, 2022

View reviewed changes

felicitymay reviewed Sep 1, 2022

View reviewed changes

aeisenberg commented Sep 2, 2022

View reviewed changes

Apply suggestions from code review

353b50b

Co-authored-by: Felicity Chapman <felicitymay@github.com> Co-authored-by: James Fletcher <42464962+jf205@users.noreply.github.com>

aeisenberg dismissed dbartol’s stale review via 353b50b September 2, 2022 20:07

felicitymay previously approved these changes Sep 5, 2022

View reviewed changes

Add reference to codeql-pack.lock.yml in test packs

9e6e37a

aeisenberg dismissed felicitymay’s stale review via 9e6e37a September 6, 2022 18:33

felicitymay approved these changes Sep 7, 2022

View reviewed changes

aeisenberg merged commit bc17d06 into main Sep 7, 2022

aeisenberg deleted the aeisenberg/about-codeql-packs branch September 7, 2022 15:05

aeisenberg added a commit that referenced this pull request Sep 9, 2022

Port Pack Docs changes to 3.7

ed66388

This moves the following three PRs to the 3.7 branch: - #10182 - #10146 - #10105

aeisenberg mentioned this pull request Sep 9, 2022

Port Pack Docs changes to 3.7 #10367

Merged

	If you do not have the CodeQL repository checked out, you can execute the same queries by specifying the query pack name and the path to the queries:
	If you do not have the CodeQL repository checked out, you can execute the same queries by specifying the query pack name and the path to the queries::


		Use the ``--download`` flag to download the query pack if it isn't yet available locally.

		After evaluating, a SARIF results file is generated. Specifying ``--format=sarif-latest`` ensures

	After evaluating, a SARIF results file is generated. Specifying ``--format=sarif-latest`` ensures
	When the analysis has finished, a SARIF results file is generated. Specifying ``--format=sarif-latest`` ensures

	Modifying an existing legacy CodeQL pack to create a CodeQL pack
	Modifying an existing legacy QL pack to create a CodeQL pack

	---------------------------------------------~~~~
	--------------------------------------------------

	If the version is excluded, then the most recent version of the pack is used.
	If you don't specify a version, then the most recent version of the pack is used.

Merge and update about-ql-packs with about-codeql-packs #10105

Merge and update about-ql-packs with about-codeql-packs #10105

Uh oh!

Conversation

aeisenberg commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aeisenberg commented Aug 19, 2022

Uh oh!

aeisenberg commented Aug 19, 2022

Uh oh!

jf205 commented Aug 22, 2022

Uh oh!

henrymercer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

henrymercer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jf205 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeisenberg commented Aug 24, 2022

Uh oh!

aeisenberg commented Aug 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dbartol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Merge and update `about-ql-packs` with `about-codeql-packs` #10105

Merge and update `about-ql-packs` with `about-codeql-packs` #10105

aeisenberg commented Aug 18, 2022 •

edited

Loading

jf205 left a comment •

edited

Loading

aeisenberg commented Aug 25, 2022 •

edited

Loading